search

NeurodataWithoutBorders / lindi

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

Refactoring #21

Closed magland closed 8 months ago

magland commented 8 months ago

There are too many changes here to review as a diff, because the classes were all renamed and reorganized.

The main change was to have the LindiH5pyFile be a subclass of h5py.File

Similarly, LindiH5pyGroup is a subclass of h5py.Group and LindiH5pyDataset is a subset of h5py.Dataset

This allows a LindiH5pyFile client to be used with pynwb just like a h5py.File client.

@rly rather than review the diff, I recommend that you look at this branch, with the updated README.md and new examples.

https://github.com/NeurodataWithoutBorders/lindi/tree/refactor

rly commented 8 months ago

pyproject.toml is set up around poetry and the CI installs the package using pip and then installs the dev dependencies separately. I'll create a PR after this is merged to make those consistent and suggest a few other nice packages for development and documentation. Do you have a preference toward pip or poetry? I slightly prefer pip because it is more common and simpler.

rly commented 8 months ago

Otherwise, this looks great to me! The code makes sense. If there are any remaining edge cases to catch, we can handle those in separate PRs/commits.

magland commented 8 months ago

pyproject.toml is set up around poetry and the CI installs the package using pip and then installs the dev dependencies separately. I'll create a PR after this is merged to make those consistent and suggest a few other nice packages for development and documentation. Do you have a preference toward pip or poetry? I slightly prefer pip because it is more common and simpler.

@rly yes I think i'd rather use pip -- a PR to clean this up is very welcome