Skip to content

Treat FSMap keys and roots as literal paths - #2224

Open
kokotatan wants to merge 1 commit into
fsspec:masterfrom
kokotatan:fix/literal-mapping-keys
Open

kokotatan wants to merge 1 commit into
fsspec:masterfrom
kokotatan:fix/literal-mapping-keys

Conversation

@kokotatan

Copy link
Copy Markdown

FSMap keys containing glob characters are currently interpreted as patterns when reading or deleting them. For example, with both chunk[0] and chunk0 present, reading chunk[0] returns the wrong file (and can return a dictionary), and deleting it can delete chunk0. The same issue affects a root such as store[0], including clear().

Use literal cat_file/rm_file operations for these paths, preserving existing bulk cat/rm calls for ordinary paths and rm-only backend compatibility for ordinary deletions. Literal bulk reads retain raise/omit/return error handling and duplicate-key behavior. Add regressions across memory, local, async wrapper, and simplecache, plus deletion/pop controls for an rm-only backend, and a changelog entry.

For roots containing glob characters, clear() removes all mapping keys while leaving any empty subdirectories. Bulk operations on paths containing glob characters run per file; ordinary bulk operations retain their existing implementation.

Validation (Windows, Python 3.13.11):

  • Exact original mapping implementation: 32 new regression cases fail, 8 literal-path controls pass; final mapping suite: 77 passed, 95% branch-inclusive module coverage.
  • Full suite before the final rm-only compatibility controls: 1,979 passed, 293 skipped, 11 xfailed, 2 deselected. The two deselections use | in local filenames and independently fail with WinError123 on unchanged tests.
  • Final full suite: 1,977 passed, 293 skipped, 11 xfailed, the same 2 deselected; 4 unchanged GitHub integration tests fail with HTTP403 API rate-limit responses from mwaskom/seaborn-data (these passed in the earlier run). Optional dependency/cloud tests account for the skips.
  • All prescribed pre-commit hooks passed; strict Sphinx HTML build passed.

AI assistance disclosure: implementation and tests prepared using OpenAI GPT-6; independent read-only review by Anthropic Claude Opus, with its compatibility finding reproduced and fixed. No human review is implied by this disclosure.

Assisted-by: OpenAI GPT-6 <noreply@openai.com>

@martindurant martindurant left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

From the very purpose of this PR, I think it is reasonable to assume that none of the inputs to a mapping have magic characters. Have you found usage where this is untrue?

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.

2 participants