search

ome / ome-zarr-py

Implementation of next-generation file format (NGFF) specifications for storing bioimaging data in the cloud.
https://pypi.org/project/ome-zarr
Other
151 stars 53 forks source link

Isn't `version` a `str`? #311

Open DimitriPapadopoulos opened 1 year ago

DimitriPapadopoulos commented 1 year ago

In the following code, the type hint for version is str. Yet, the code has been modified to handle floats too (#189).

Should the type hint be changed from str to str | float?

https://github.com/ome/ome-zarr-py/blob/6a1de2a0924e821fecec7ac3ba8790c0a3ae234f/ome_zarr/format.py#L12-L16

DimitriPapadopoulos commented 1 year ago

And here, the type hint for well is str. Why use str(well)? Defensive programming?

https://github.com/ome/ome-zarr-py/blob/6a1de2a0924e821fecec7ac3ba8790c0a3ae234f/ome_zarr/format.py#L137-L140

DimitriPapadopoulos commented 1 year ago

Isn't parent_dir a str? It could be perhaps None (if input_path is empty or doesn't contain a slash) , but do you really want to use "None" in that corner case? https://github.com/ome/ome-zarr-py/blob/6a1de2a0924e821fecec7ac3ba8790c0a3ae234f/ome_zarr/utils.py#L58-L59