Skip to content

[WIP] [CF-931] Implement optional filesystem argument in cfdm.read#205

Closed
Copilot wants to merge 1 commit intomainfrom
copilot/implement-filesystem-argument-cfdm-read
Closed

[WIP] [CF-931] Implement optional filesystem argument in cfdm.read#205
Copilot wants to merge 1 commit intomainfrom
copilot/implement-filesystem-argument-cfdm-read

Conversation

Copy link

Copilot AI commented Mar 16, 2026

Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.

Title: cfdm implementation PR

Repository: NCAS-CMS/cfdm

Task:
Implement support for a new optional filesystem keyword argument in cfdm.read, so callers can pass a pre-authenticated filesystem object (for example fsspec filesystems).

Issue context:
This work supports NCAS-CMS/cf-python issue 931 and should enable non-native URI backends (for example ssh/sftp/http-range) via file-like objects from filesystem.open(path, "rb").

Required behavior:

  1. Add filesystem=None to cfdm.read public signature.
  2. When filesystem is provided:
  • Bypass local string preprocessing/globbing/walk logic in _datasets.
  • Treat dataset values as paths understood by the provided filesystem.
  1. In netcdf read open path:
  • Prefer filesystem.open(dataset, "rb") to create file-like object.
  • Pass this file-like object into existing backend open flow.
  1. Preserve existing behavior when filesystem is None.
  2. Do not break current storage_options logic.
  3. Keep this read-only scope (no write-path changes).

Files likely to change:

  • cfdm/read_write/read.py
  • cfdm/read_write/netcdf/netcdfread.py
  • cfdm/docstring/docstring.py
  • any docs source needed for read parameter text
  • tests in cfdm test suite

Testing requirements:

  1. Add unit tests verifying filesystem.open is used.
  2. Add unit tests verifying local globbing expansion is bypassed when filesystem is provided.
  3. Ensure existing tests still pass.
  4. Report exact test commands and results in the PR description.

Constraints:

  1. Minimal diff, no unrelated refactors.
  2. Keep backward compatibility.
  3. If both filesystem and storage_options are provided, do not break existing callers; prefer filesystem open-path behavior for actual file opening.
  4. Show me final diff summary before opening PR.

PR requirements:

  1. Open PR against main.
  2. Reference issue 931 in PR body.
  3. Include:
  • motivation
  • behavior changes
  • backward compatibility statement
  • test evidence

Stop points:
If anything is ambiguous, make a conservative choice and document it in PR notes.


💬 Send tasks to Copilot coding agent from Slack and Teams to turn conversations into code. Copilot posts an update in your thread when it's finished.

@valeriupredoi
Copy link
Collaborator

valeriupredoi commented Mar 17, 2026

@bnlawrence do you want Copilot to go about and open PRs on its own? I don't particularly like this behaviour to be honest - also, why is it opening a PR here, when the work should be in cfdm? 🍺

@valeriupredoi
Copy link
Collaborator

@copilot why are you opening a PR here, when the work will be done in cfdm?

@bnlawrence
Copy link
Collaborator

bnlawrence commented Mar 17, 2026

This was absolutely my fault and not copilots! Although if it had been a bit cleverer it would have realised I had been stupid.

@bnlawrence
Copy link
Collaborator

Complete stuff up.

@bnlawrence bnlawrence closed this Mar 17, 2026
@valeriupredoi
Copy link
Collaborator

no worries, @bnlawrence I thought we already had mini-Skynet on our hands

@valeriupredoi valeriupredoi deleted the copilot/implement-filesystem-argument-cfdm-read branch March 17, 2026 14:01
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.

3 participants