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.

Sign up for GitHub

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

Jump to bottom

55 from ds #56

Merged

rogerkuou merged 8 commits into main from 55_from_zarr

Oct 14, 2024

Member

rogerkuou commented Oct 1, 2024 •

edited

Loading

Fix #55.

Add from_zarr function
Add test files and unit tests
Replace lint unit test with precommit hooks
Remove example dir from ruff check

rogerkuou added 6 commits

October 1, 2024 15:34


          add from_zarr function

ac15b96


          fix test files

3fc99f5


          add tests

59b56c3


          formatting

b37f086


          replace lint unit test with pre-commit hook

8fb668c


          remove examples from ruff lint check

5ff6b14

Member Author

rogerkuou commented Oct 1, 2024

Hi @SarahAlidoost, when you have time, can you review this PR? The details should be in the issue description.

While creating this PR, the ruff linting is failling on the notebooks. These notebooks exists mainly for archiving purposes. In the future they will all be moved to PyDePSI workflow. Therefore I turned off ruff linting on them. I also replaced the unit test for linting with a pre-commit hook.

rogerkuou requested a review from SarahAlidoost

October 1, 2024 14:44

SarahAlidoost reviewed

View reviewed changes

sarxarray/_io.py Outdated Show resolved Hide resolved

SarahAlidoost reviewed

View reviewed changes

sarxarray/_io.py Outdated Show resolved Hide resolved

SarahAlidoost reviewed

View reviewed changes

sarxarray/_io.py Outdated Show resolved Hide resolved

SarahAlidoost reviewed

View reviewed changes

sarxarray/_io.py Outdated Show resolved Hide resolved

SarahAlidoost reviewed

View reviewed changes

sarxarray/_io.py Outdated Show resolved Hide resolved

SarahAlidoost reviewed

View reviewed changes

tests/test_io.py Outdated Show resolved Hide resolved

SarahAlidoost reviewed

View reviewed changes

tests/test_io.py Outdated Show resolved Hide resolved

SarahAlidoost reviewed

View reviewed changes

tests/test_io.py Outdated Show resolved Hide resolved

SarahAlidoost requested changes

View reviewed changes

Member

SarahAlidoost left a comment

@rogerkuou please see my comments and suggestions, the main concern is this comment.

rogerkuou and others added 2 commits

October 10, 2024 15:15


          Apply suggestions from code review

702a783

Co-authored-by: SarahAlidoost <[email protected]>


          rename from_zarr to from_ds

6bfd60a

rogerkuou changed the title ~~55 from zarr~~ 55 from ds

Member Author

rogerkuou commented Oct 11, 2024

Thanks @SarahAlidoost! I adpted your suggestions and re-write from_zarr to from_ds. Now the reading function operates on a DS read by open_zarr. Can you give it another look?

SarahAlidoost approved these changes

View reviewed changes

Member

SarahAlidoost left a comment

@rogerkuou thanks for the fixes, looks great 👍

rogerkuou merged commit fc04d1f into main

15 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet