-
Notifications
You must be signed in to change notification settings - Fork 224
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
List key development dependencies to install for new contributors #1783
Conversation
Namely git and dvc for code and data version control, pytest-mpl for testing baseline images, and sphinx-gallery for building the gallery example page.
@@ -192,8 +192,16 @@ It will make your life a lot easier! | |||
|
|||
The repository includes a conda environment file `environment.yml` with the | |||
specification for all development requirements to build and test the project. | |||
In particular, these are some of the key development dependencies you will need | |||
to install to build the documentation and run the unit tests locally: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Building the documentation needs more packages (like sphinx) than listed here.
The repository includes a conda environment file `environment.yml` with the | ||
specification for all development requirements to build and test the project. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The repository includes a conda environment file
environment.yml
with the
specification for all development requirements to build and test the project.
Actually I feel this sentence is very clear that all dependencies are listed in the environment.yml and people who want to build and test should read the environment.yml file, rather than listing an incompleted list here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A compromise could be to add more sections (via comments) to
environment.yml
that specify which development dependencies are required for building the docs, running the tests, and/or formatting the code (inspiration from numpy and pandas environment files).
Yes, that sounds like a good idea actually.
So do we not want to list some of the dev dependencies here in contributing.md? I know it seems redundant, but there are some people who might be from a non-Anaconda/pure-Python-pip sort of world that don't really use that environment.yml file. Listing things like git/dvc/pytest/sphinx here up front will at least give people an idea on what framework PyGMT uses for testing and building documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A compromise could be to add more sections (via comments) to
environment.yml
that specify which development dependencies are required for building the docs, running the tests, and/or formatting the code (inspiration from numpy and pandas environment files).Yes, that sounds like a good idea actually.
Yes, I agree.
Listing things like git/dvc/pytest/sphinx here up front will at least give people an idea on what framework PyGMT uses for testing and building documentation.
Fair point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, environment.yml
has been reorganized into subsections (general, linters, testing, documentation) in ab8ac74.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this paragraph also include a short sentence on how to install dependencies with a .yml file? Since it's different than the typical pip install -r requirements.txt
, it would cut down on confusion of how to use environment.yml
to install dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. There's actually a conda env create
instruction just a few sentences below. I've just added a commit (c2ae7dd) to make it more explicit (conda env create --file environment.yml
).
Including subsections for general development dependencies, style checks, unit testing, and building documentation.
@@ -13,22 +13,25 @@ dependencies: | |||
- packaging |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line 7: can we remove pip
from the list?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could, but then people might get a warning that says: "Warning : you have pip-installed dependencies in your environment file, but you do not list pip itself as one of your conda dependencies...". See https://stackoverflow.com/questions/58544099/warning-after-i-run-the-command-conda-env-create-f-environment-yml.
Edit: Oh wait, actually that warning won't show up since we don't list pip dependencies, but I think listing it explicitly doesn't really hurt?
…nericMappingTools#1783) * List some of the key dev dependencies for developing pygmt locally * Reorganize development dependencies in environment.yml into subsections * Explicitly use conda env create --file environment.yml
Description of proposed changes
As a new contributor, it's not obvious what packages are needed to help out with PyGMT development locally. This Pull Request attempts to clarify that, by listing some of the key dependencies needed so that
make test
andcd doc && make all
won't result in nasty errors.Note that this is one of the other major points from the PyOpenSci review we'll need to handle, so give it a bit of thought 🙂
Fixes #1697
Reminders
make format
andmake check
to make sure the code follows the style guide.doc/api/index.rst
.Slash Commands
You can write slash commands (
/command
) in the first line of a comment to performspecific operations. Supported slash commands are:
/format
: automatically format and lint the code/test-gmt-dev
: run full tests on the latest GMT development version