Skip to content

Add FCPath support#10

Merged
rfrenchseti merged 2 commits intomainfrom
rf_251125_filecache
Dec 2, 2025
Merged

Add FCPath support#10
rfrenchseti merged 2 commits intomainfrom
rf_251125_filecache

Conversation

@rfrenchseti
Copy link
Copy Markdown
Collaborator

@rfrenchseti rfrenchseti commented Nov 26, 2025

Fixed Issues

Summary of Changes

  • Adds support for FCPath to allow reading a remote kernel file.

Known Problems

  • None.

Summary by CodeRabbit

  • Chores

    • Removed Python 3.8; minimum Python is now 3.9.
    • Added rms-filecache and rms-julian runtime dependencies.
  • New Features

    • File path handling expanded to accept additional URL formats and sources.
  • Documentation

    • Clarified supported input path types and value formats (ints, floats, strings, datetime objects, and lists).

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Nov 26, 2025

Walkthrough

Removed Python 3.8 support from CI and project metadata, added rms-filecache (and rms-julian in pyproject) as dependencies, updated README to document FCPath-supported input paths and broadened allowed value types, and replaced pathlib-based file handling with filecache.FCPath in textkernel.from_file.

Changes

Cohort / File(s) Summary
CI & Project Metadata
/.github/workflows/run-tests.yml, pyproject.toml, requirements.txt
Removed Python 3.8 from the test matrix and project classifiers; raised minimum Python requirement to >=3.9 in pyproject.toml. Added rms-filecache (and rms-julian in pyproject.toml) as new runtime dependencies; added rms-filecache to requirements.txt.
Documentation
README.md
Added notes that from_file accepts any URL supported by FCPath and clarified tkdict value types to include Python ints, floats, strings, datetime objects, or lists containing these types; these notes were added after the simple usage example and within the main values description.
Core Implementation
textkernel/__init__.py
Replaced pathlib usage with filecache.FCPath in from_file (import adjusted); reading now uses filecache.FCPath(path).read_text(...). Docstring updated to list FCPath-compatible inputs; public signature unchanged.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect textkernel/__init__.py to ensure FCPath usage preserves behavior for local paths and supports remote URLs (encoding, errors).
  • Verify dependency additions (rms-filecache, rms-julian) in pyproject.toml are intentional and check for version constraints or compatibility.
  • Confirm CI matrix and classifiers consistently reflect the raised minimum Python version and that tests run under the new matrix.

Pre-merge checks

✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Add FCPath support' clearly summarizes the main change: adding FCPath (from filecache) support to enable reading remote kernel files.
Description check ✅ Passed The description follows the required template with Fixed Issues, Summary of Changes, and Known Problems sections all completed appropriately.
Linked Issues check ✅ Passed The PR successfully implements support for filecache (FCPath) as required by issue #9, enabling text kernels to be read via filecache for cloud operations.
Out of Scope Changes check ✅ Passed All changes are within scope: FCPath integration in code, documentation updates, Python 3.8 removal, and new dependency additions directly support the filecache support objective.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Nov 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (9f4890f) to head (484555d).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #10   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines          326       326           
  Branches        67        67           
=========================================
  Hits           326       326           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Collaborator

@markshowalter markshowalter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please make the change "latin8" to "latin1". Then you can squash and merge.

"""

text = pathlib.Path(path).read_text(encoding='latin8')
text = filecache.FCPath(path).read_text(encoding='latin8')
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an older bug. Should be "latin1", not "latin8". Please fix.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 306d20f and 484555d.

📒 Files selected for processing (1)
  • textkernel/__init__.py (3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Test textkernel (windows-latest, 3.10)
🔇 Additional comments (1)
textkernel/__init__.py (1)

310-311: LGTM! Clear documentation of FCPath support.

The docstring accurately documents that FCPath is now a supported path type alongside strings and Path objects.

@rfrenchseti rfrenchseti merged commit bfc8607 into main Dec 2, 2025
17 checks passed
@rfrenchseti rfrenchseti deleted the rf_251125_filecache branch December 2, 2025 19:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for filecache

2 participants