fix(nanoevents): from_parquet accepts Path/ParquetFile/file-like inputs again

[NJManganelli]

Part of #1578 — bug 14: from_parquet crashes for most input types.

NanoEventsFactory.from_parquet documents str, pathlib.Path, pyarrow.NativeFile/io.IOBase file-like objects, and pyarrow.parquet.ParquetFile as valid inputs, but only the str branch assigned the local fs_file, which is unconditionally forwarded to the parquet shim's openfile= argument. Every non-str input type therefore crashed with UnboundLocalError, leaving str as the only working path.

This initializes fs_file = None ahead of the input-type dispatch, so the fsspec-materialized handle is only set (and later closed) for the str path while other input types pass openfile=None. A parametrized regression test covers all four documented input types in both eager and virtual modes and asserts they yield events identical to the str path (8 passed with the fix; 6 of 8 fail without it). Full tests/test_nanoevents.py passes; pre-commit clean.

🤖 Generated with Claude Code

[lgray]

🤖 AI text below 🤖

Summary

This is a clean, correct, and well-tested one-line bug fix. from_parquet documents str, pathlib.Path, pyarrow.NativeFile/io.IOBase file-likes, and pyarrow.parquet.ParquetFile as inputs, but only the str branch assigned fs_file, which is unconditionally forwarded to the shim as openfile= (factory.py:600). Every non-str input therefore crashed with UnboundLocalError. Initializing fs_file = None before the dispatch (factory.py:559) fixes it. I verified the fix is genuinely correct downstream, not just papering over the crash: the shim's read() pulls from self.file (the ParquetFile), and openfile is dereferenced only in a None-guarded __del__ (mapping/parquet.py:35-36), so openfile=None reads correctly and never closes a caller-owned handle.

Overall assessment: approve. The change is minimal, root-caused in the single shared function all callers route through, and the regression test is non-vacuous and discriminating.

Verification performed in an isolated venv:

• New test: 8 passed; full test_nanoevents.py: 26 passed, 1 skipped (skip unrelated — no distributed).
• Discriminating proof: reverting only factory.py to master makes exactly 6 of 8 fail with UnboundLocalError, the 2 survivors being both str params — matches the PR description exactly.
• Independent equality probe (eager): str, Path, ParquetFile, an io file object, and a pyarrow.OSFile NativeFile all produce events byte-identical to the str path (fields, full Muon.pt, per-event Jet counts, length=40).

Test coverage

Strong. The test is proven discriminating (fails 6/8 on master) and asserts content equality to the str reference (ak.to_list(events.Muon.pt) == ref_pt), not just absence of exceptions. It covers all documented types × eager/virtual, constructs the file object the way a user would (open(path, "rb")), and correctly handles the virtual case via ak.materialize inside the with block. Dask mode is correctly excluded (it raises NotImplementedError with a schema regardless of input type).

Nits / Optional

• Docstring omits ParquetFile. The file param docstring (factory.py:481) lists "str or pathlib.Path or pyarrow.NativeFile or io.IOBase" but not pyarrow.parquet.ParquetFile, even though it's an accepted dispatch branch (factory.py:568) and now tested. This is pre-existing, not introduced here, but since the PR is about aligning documented-vs-accepted inputs, adding it to the docstring would close the last asymmetry.
• Test's file-like coverage could note NativeFile. The matrix uses a Python io object for fileobj; pyarrow.NativeFile (e.g. pyarrow.OSFile) travels the same ftypes branch, so it's covered transitively — I confirmed it works independently. No change needed; just noting the branch is exercised.

Pydantic

Not applicable — the diff touches no pydantic code (grep -ci pydantic on the diff = 0).

Nice, tightly-scoped fix with a genuinely discriminating regression test.

[NJManganelli]

Good to go?

[ikrommyd]

I'm tempted to allow path-like objects in general and extend that to from_root too? What do you think? Can be done with os.fspath which supports all path-like objects (include __fspath__)

[NJManganelli]

I’m fine with extensions, what’s the venn diagram with upaths, which ServiceX folks were interested in to support token auth forwarding appropriately? Best, Nick

…

On Sun, Jul 12, 2026 at 20:51 Iason Krommydas ***@***.***> wrote: *ikrommyd* left a comment (scikit-hep/coffea#1590) <#1590 (comment)> I'm tempted to allow path-like objects in general and extend that to from_root too? What do you think? Can be done with os.fspath <https://docs.python.org/3/library/os.html#os.fspath> which supports all path-like objects (include __fspath__) — Reply to this email directly, view it on GitHub <#1590?email_source=notifications&email_token=AJDSMOVS4LYCAG35QPB2XDL5EPM4PA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJVGIZTKMRRHA22M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4952352185>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJDSMOSVB3MIEABITVQN4AT5EPM4PAVCNFSNUABFKJSXA33TNF2G64TZHMYTKOJWG4ZTCMZZHNEXG43VMU5TIOBRGUZDMNRUGU2KC5QC> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[nsmith-]

Ah, so upath allows to embed the storage_options in a single object? That seems very useful.

[NJManganelli]

I threw claude at this too, but have not opened the results yet, once I do I’ll push as a draft (but I have to have the pydantic PR merged, it’s a base for 5 extensions). I should have a lot to cover next coffea meeting

…

On Tue, Jul 14, 2026 at 16:23 Nicholas Smith ***@***.***> wrote: *nsmith-* left a comment (scikit-hep/coffea#1590) <#1590 (comment)> Ah, so upath allows to embed the storage_options in a single object? That seems very useful. — Reply to this email directly, view it on GitHub <#1590?email_source=notifications&email_token=AJDSMOSBK4ZPBBIHZACPPGT5EY65VA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJXGAZDMMRUG422M4TFMFZW63VGMF2XI2DPOKSWK5TFNZ2KYZTPN52GK4S7MNWGSY3L#issuecomment-4970262475>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AJDSMORIQUI4FAPA4CZ6EML5EY65VAVCNFSNUABFKJSXA33TNF2G64TZHMYTKOJWG4ZTCMZZHNEXG43VMU5TIOBRGUZDMNRUGU2KC5QC> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[NJManganelli]

This PR, however, is straightforward, can we merge? Let's leave upath and other extensions to their own PR(s). @lgray @ikrommyd @nsmith-