Skip to content

Docstrings for arraymodule #2276

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

Merged
merged 11 commits into from
Oct 15, 2024
Merged

Docstrings for arraymodule #2276

merged 11 commits into from
Oct 15, 2024

Conversation

e-marshall
Copy link
Contributor

@e-marshall e-marshall commented Sep 30, 2024

[Description of PR]
This is a draft PR that adds docstrings and a few code examples to the Array module.
It's not complete, but I probably won't have more time to work on this today. I'd also love to check in and make sure things look okay. Any feedback or suggestions are very welcome!

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)

Copy link
Contributor

@TomAugspurger TomAugspurger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks! Just a couple small notes.

@e-marshall
Copy link
Contributor Author

Thanks so much @TomAugspurger !

@jhamman jhamman added documentation Improvements to the documentation V3 labels Oct 7, 2024
@jhamman
Copy link
Member

jhamman commented Oct 7, 2024

@e-marshall - this is still marked as a "work in progress" -- is it ready for a review?

@e-marshall
Copy link
Contributor Author

sorry, got sidetrackd last week! Thanks for the nudge @jhamman , this is ready for review

@e-marshall e-marshall marked this pull request as ready for review October 7, 2024 20:29
@e-marshall e-marshall changed the title start to docstrings for arraymodule Docstrings for arraymodule Oct 7, 2024
Copy link
Contributor

@dstansby dstansby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left one comment for a section that can be removed. This also needs a rebase to fix the merge conflict. Otherwise, at a glance it looks good to me 👍

@jhamman jhamman changed the base branch from v3 to main October 14, 2024 20:51
@dstansby
Copy link
Contributor

Thanks a lot! I started reading the docstrings, but don't think I have the technical knowledge of the v3 codebase to properly review them, so I will leave it to someone else to do that

@jhamman jhamman merged commit 1131253 into zarr-developers:main Oct 15, 2024
20 checks passed
@e-marshall e-marshall deleted the docs_v3 branch October 16, 2024 00:28
d-v-b pushed a commit to d-v-b/zarr-python that referenced this pull request Oct 18, 2024
* start to docstrings for arraymodule

* incorporating toms edits, overriding mypy error...

* fix attrs

* Update src/zarr/core/array.py

Co-authored-by: Sanket Verma <[email protected]>

* fix store -> storage

* remove properties from asyncarray docstring

---------

Co-authored-by: Sanket Verma <[email protected]>
Co-authored-by: Joe Hamman <[email protected]>
d-v-b added a commit that referenced this pull request Oct 18, 2024
* move v3/tests to tests and fix various mypy issues

* test(ci): change branch name in v3 workflows (#2368)

* Use lazy % formatting in logging functions (#2366)

* Use lazy % formatting in logging functions

* f-string should be more efficient

* Space before unit symbol

From "SI Unit rules and style conventions":
https://physics.nist.gov/cuu/Units/checklist.html

	There is a space between the numerical value and unit symbol,
	even when the value is used in an adjectival sense, except in
	the case of superscript units for plane angle.

* Enforce ruff/flake8-logging-format rules (G)

---------

Co-authored-by: Joe Hamman <[email protected]>

* Move roadmap and v3-design documument to docs (#2354)

* move roadmap to docs

* formatting and minor copy editing

* Multiple imports for an import name (#2367)

Co-authored-by: Joe Hamman <[email protected]>

* Enforce ruff/pycodestyle warnings (W) (#2369)

* Apply ruff/pycodestyle rule W291

W291 Trailing whitespace

* Enforce ruff/pycodestyle warnings (W)

It looks like `ruff format` does not catch all trailing spaces.

---------

Co-authored-by: Joe Hamman <[email protected]>

* Apply ruff/pycodestyle preview rule E262 (#2370)

E262 Inline comment should start with `# `

Co-authored-by: Joe Hamman <[email protected]>

* Fix typo (#2382)

Co-authored-by: Joe Hamman <[email protected]>

* Imported name is not used anywhere in the module (#2379)

* Missing mandatory keyword argument `shape` (#2376)

* Update ruff rules to ignore (#2374)

Co-authored-by: Joe Hamman <[email protected]>

* Docstrings for arraymodule (#2276)

* start to docstrings for arraymodule

* incorporating toms edits, overriding mypy error...

* fix attrs

* Update src/zarr/core/array.py

Co-authored-by: Sanket Verma <[email protected]>

* fix store -> storage

* remove properties from asyncarray docstring

---------

Co-authored-by: Sanket Verma <[email protected]>
Co-authored-by: Joe Hamman <[email protected]>

* fix/normalize storage paths (#2384)

* bring in path normalization function from v2, and add a failing test

* rephrase comment

* simplify storepath creation

* Update tests/v3/test_api.py

Co-authored-by: Joe Hamman <[email protected]>

* refactor: remove redundant zarr format fixture

* replace assertion with an informative error message

* fix incorrect path concatenation in make_store_path, and refactor store_path tests

* remove upath import because we don't need it

* apply suggestions from code review

---------

Co-authored-by: Joe Hamman <[email protected]>

* Enforce ruff/flake8-pyi rule PYI013 (#2389)

PYI013 Non-empty class body must not contain `...`

Note that documentation is enough to fill the class body.

* deps: remove fasteners from list of dependencies (#2386)

* Enforce ruff/flake8-annotations rule ANN003 (#2388)

ANN003 Missing type annotation

Co-authored-by: Joe Hamman <[email protected]>

* Enforce ruff/Perflint rules (PERF) (#2372)

* Apply ruff/Perflint rule PERF401

PERF401 Use a list comprehension to create a transformed list

* Enforce ruff/Perflint rules (PERF)

* chore: update package maintainers (#2387)

* chore: update package maintainers

* Update pyproject.toml

Co-authored-by: David Stansby <[email protected]>

---------

Co-authored-by: David Stansby <[email protected]>

* Fixed consolidated Group getitem with multi-part key (#2363)

* Fixed consolidated Group getitem with multi-part key

This fixes `Group.__getitem__` when indexing with a key
like 'subgroup/array'. The basic idea is to rewrite the indexing
operation as `group['subgroup']['array']` by splitting the key
and doing each operation independently.

Closes #2358

---------

Co-authored-by: Joe Hamman <[email protected]>

* chore: add python 3.13 to ci / pyproject.toml (#2385)

* chore: add python 3.13 to ci / pyproject.toml

* update hatch matrix

* remove references to dead test dir in pyproject.toml

* remove v3 reference in test

---------

Co-authored-by: Joe Hamman <[email protected]>
Co-authored-by: Dimitri Papadopoulos Orfanos <[email protected]>
Co-authored-by: Emma Marshall <[email protected]>
Co-authored-by: Sanket Verma <[email protected]>
Co-authored-by: David Stansby <[email protected]>
Co-authored-by: Tom Augspurger <[email protected]>
@jhamman jhamman mentioned this pull request Nov 29, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants