Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactoring #21

Merged
merged 32 commits into from
Mar 21, 2024
Merged

Refactoring #21

merged 32 commits into from
Mar 21, 2024

Conversation

magland
Copy link
Collaborator

@magland magland commented Mar 20, 2024

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
Copy link
Contributor

rly commented Mar 21, 2024

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
Copy link
Contributor

rly commented Mar 21, 2024

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
Copy link
Collaborator Author

magland commented Mar 21, 2024

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

@magland magland merged commit 76925be into main Mar 21, 2024
4 checks passed
@magland magland deleted the refactor branch March 21, 2024 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants