Issue fixed

[h20200051]

Add API methods proposed in #1113

TODO:

• Add unit tests and/or doctests in docstrings
• Add docstrings and API docs for any new/modified user-facing classes and functions
• New/modified features documented in docs/tutorial.rst
• Changes documented in docs/release.rst
• GitHub Actions have all passed
• Test coverage is 100% (Codecov passes)

[h20200051]

@MSanKeys963 I have sent the draft pull request for review.Kindly review it so that bugs and errors can be fixed

[rabernat]

Thanks for your contribution to Zarr. Could you please fill in the PR description and mark the appropriate check boxes. It's not clear to us what the PR is for. Adding new features to core.py is a pretty big change that will need careful review. Adding h5py and kerchunk as required dependencies is out of the question.

[h20200051]

@rabernat I have raised the PR for issueno_1113.

[joshmoore]

@h20200051, looking at #1113 (and the related repo which is now called "PyActiveStorage") I agree with @rabernat that these aren't methods that will be able to live in zarr/core.

[MSanKeys963]

Hi @h20200051. Thanks for sending the PR.

Apologies for the late response. Here are my thoughts:

Issue #1113 refers to exposing additional chunk(s)/slice (like byte offset and size) apart from what's already in core.py and making it available to the user.

The script chunk_slices.py that you've used has additional dependencies and code which is not essential and certainly should not be in the core.py. For, e.g. I don't think we need def build_zarr_dataset() as this is already been taken care of in creation.py.

Also, as mentioned by @rabernat, I'd suggest removing h5py and kerchunk as the required dependencies. In simple words, any new imports added in core.py act as a required dependency and must be carefully reviewed before merging.

Here's what you can do next:

• I can see using the PartialChunkIterator to achieve some of the functionality from chunk_slices.py.

• Please take this with a pinch of salt: If there's a strong need to use h5py and kerchunk to add the additional chunk(s) info, then I can see using them as an optional dependency and only invoking them when the user requests for additional chunk(s) info.

Let us know if there are any more questions, and I look forward to your contributions to Zarr. Also, I think these discussions are why we hold bi-weekly community calls, and I'd like to invite you to the next Zarr Community Meeting on 1st December. The meeting info is here: https://zarr.dev/community-calls/.

[h20200051]

@MSanKeys963 I have made the changes which was suggested by you.Could you plz take a look and suggest any changes if required

[lgtm-com]

This pull request introduces 1 alert when merging d1594ce into e4668e0 - view on LGTM.com

new alerts:

• 1 for Syntax error

---

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

[h20200051]

hello guys I would request you to take a look so that issue_1113
can be resolved asap.All the test cases have been passed from my side

[h20200051]

@MSanKeys963 @rabernat @joshmoore hello guys,can you plz review the code and suggest the changes so that remaining checklist will become successful.I am very close to solving this issue so that it can be merged I request you to help me so that it can be done asap

[MSanKeys963]

Hi again, @h20200051. Thanks for working through this PR.

After going through the code in the PR, it seems like there are still inconsistencies in the code, and it would not integrate well with the core.py module.
To check things further, I even tried to run the test on my machine, and the tests in the Zarr suite are not passing successfully.

You can try running the tests by setting up the dev environment and running python -m pytest -v zarr to check yourself. Please refer to this for setting up the dev environment and running tests in the Zarr suite using the above command: https://zarr.readthedocs.io/en/stable/contributing.html

Also, I think there might be another way to integrate the functionality in this PR rather than changing the core.py. I understand that you've spent significant time working on the issue, and the community appreciate it. Why don't you join the next community meeting on 1st December so that we can think this through?
The meeting info is here: https://zarr.dev/community-calls/.

Also, we'd very much like you to keep contributing to the Zarr, and I'd like to draw your attention to issues which need help. E.g.: good-first-issues, help-wanted and documentation issues.

Let us know if there are any other issues. Thanks!

[joshmoore]

Thanks again for working on this, @h20200051. I'm going to close it for now, but if you would like to give it another try, please let us know.