Fix bug in RT of parquet detection

[norlandrhagen]

No description provided.

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 67.92%. Comparing base (fffdc2d) to head (56cb1c4).

Files with missing lines	Patch %	Lines
virtualizarr/readers/kerchunk.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #278   +/-   ##
=======================================
  Coverage   67.92%   67.92%           
=======================================
  Files          41       41           
  Lines        2516     2516           
=======================================
  Hits         1709     1709           
  Misses        807      807           

Flag	Coverage Δ
unittests	67.92% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

[norlandrhagen]

Typing: #274

[keewis]

according to fsspec/kerchunk#519 (comment), you could also check for the presence of f"{fs.filepath}/.zmetadata", which should avoid trying to open actual parquet files – the only downside is a additional request, but since we already check magic bytes elsewhere I don't think that's a concern:

Suggested change

	if fs.filepath.endswith(".parquet"):
	if fs.filepath.endswith(".parquet") and fs.fs.isfile(f"{fs.filepath}/.zmetadata"):

(or define isfile / exists on _FsspecFSFromFilepath and use that instead of fs.fs.isfile)

Also, do we want to support .parq, as well, or do we expect people to always end the filename with .parquet? If we do want to support .parq, you can use fs.filepath.endswith((".parquet", ".parq")) (not that I'd need that myself).

cc @martindurant

[norlandrhagen]

Thanks @keewis!

Also, do we want to support .parq, as well, or do we expect people to always end the filename with .parquet? If we do want to support .parq, you can use fs.filepath.endswith((".parquet", ".parq")) (not that I'd need that myself).

I don't have really strong opinions on this. Adding more file name guessing doesn't feel great to me. I could see people naming files with a .pqt suffix since it's really just a directory name we're looking at. Maybe we just stick with .parquet and mention in the docs and/or in the ValueError?

cc @TomNicholas

[keewis]

fine with me!

[martindurant]

I don't have much of an opinion. Explicit is better than guessing, but clear documentation and error message either way.

[norlandrhagen]

Thanks both! I added a bit to the error message.