NeurodataWithoutBorders / lindi

Linked Data Interface (LINDI) - cloud-friendly access to NWB data
BSD 3-Clause "New" or "Revised" License
2 stars 1 forks source link

Implement `LindiReferenceFileSystemStore.__contains__` #75

Closed rly closed 4 months ago

rly commented 4 months ago

MutableMapping implements __contains__ by calling __getitem__ and checking for None. Zarr BaseStore has this line: https://github.com/zarr-developers/zarr-python/blob/b1f4c509abaee1cb8dec18e3a973e1199226011a/src/zarr/v2/_storage/store.py#L142

which does a both __contains__ check and __getitem__ check. That results in two remote requests per __getitem__ call in LindiReferenceFileSystemStore.

This PR removes the remote request for __contains__.

codecov-commenter commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 79.43%. Comparing base (5922b4f) to head (4fdab5f).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #75 +/- ## ========================================== + Coverage 79.41% 79.43% +0.01% ========================================== Files 30 30 Lines 2254 2256 +2 ========================================== + Hits 1790 1792 +2 Misses 464 464 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

magland commented 4 months ago

Nice catch!

rly commented 4 months ago

FYI I released this on PyPI as 0.3.8.