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

DOC: Proposed updates to contributor guide. #2513

Merged
merged 3 commits into from
Dec 5, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions docs/contributing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,11 @@ the following::
$ mkdir -p ~/pyenv/zarr-dev
$ python -m venv ~/pyenv/zarr-dev
$ source ~/pyenv/zarr-dev/bin/activate
$ pip install -r requirements_dev_minimal.txt -r requirements_dev_numpy.txt
$ pip install -e .[docs]
$ pip install -e .[test,docs]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There don't appear to be requirements files anymore, so I assume that the optional-dependencies is the intended replacement - not sure how these intersect requirements_dev_minimal and requirements_dev_numpy but these appear to be the minimal ones to run the tests and build the docs!

Copy link
Member

Choose a reason for hiding this comment

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

We are using hatch now to manage our development environments. I think we should replace the virtual env directions with instructions on how to use hatch.


To verify that your development environment is working, you can run the unit tests::

$ python -m pytest -v zarr
$ python -m pytest -v tests
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like there's been a src/tests split since this was written, so the original recommendation no longer applies. This should work and be portable, but feel free to replace with whichever incantation is preferred!

Copy link
Member

Choose a reason for hiding this comment

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

I think the standard invocation is hatch env run run (perhaps we should change this to hatch env run tests?)


Creating a branch
~~~~~~~~~~~~~~~~~
Expand Down Expand Up @@ -149,7 +148,7 @@ and invoke::
Some tests require optional dependencies to be installed, otherwise
the tests will be skipped. To install all optional dependencies, run::

$ pip install -r requirements_dev_optional.txt
$ pip install pytest-doctestplus
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't see pytest-doctestplus listed anywhere in the optional dependencies, and I don't know what else may have been in requirements_dev_optional.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be pro putting this in the test dependencies section, and then not having any optional dependencies for testing? That way it's a bit simpler.


To also run the doctests within docstrings (requires optional
dependencies to be installed), run::
Expand Down Expand Up @@ -234,7 +233,7 @@ should run and pass as doctests under Python 3.8. To run doctests,
activate your development environment, install optional requirements,
and run::

$ python -m pytest -v --doctest-plus zarr
$ python -m pytest -v --doctest-plus tests

Zarr uses Sphinx for documentation, hosted on readthedocs.org. Documentation is
written in the RestructuredText markup language (.rst files) in the ``docs`` folder.
Expand Down
Loading