Update contributing guide, fail fast #4280
Conversation
MarcoGorelli
force-pushed
the
update-contributing
branch
from
83d6d96 to
441828d
Compare
| conda activate testenv | ||
| conda remove arviz -y | ||
| pip install -e . |
There was a problem hiding this comment.
it's probably good to check this works, in case there are any errors in setup.py - it only takes about 5 seconds during CI anyway
CONTRIBUTING.md
Outdated
|
|
||
| ```bash | ||
| $ conda env create -f environment-dev.yml | ||
| $ conda activate testenv |
There was a problem hiding this comment.
I think we can use a more explicit name for the virtual env, maybe pymc-dev-env instead of testenv?
There was a problem hiding this comment.
or just pymc3-dev - sure!
Codecov Report
@@ Coverage Diff @@
## master #4280 +/- ##
==========================================
- Coverage 87.61% 87.59% -0.03%
==========================================
Files 88 88
Lines 14312 14312
==========================================
- Hits 12539 12536 -3
- Misses 1773 1776 +3
|
| @@ -38,19 +38,27 @@ The preferred workflow for contributing to PyMC3 is to fork the [GitHub reposito | |||
| 3. Create a ``feature`` branch to hold your development changes: | |||
|
|
|||
| ```bash | |||
| $ git checkout -b my-feature | |||
| $ git switch -c my-feature | |||
There was a problem hiding this comment.
git's new way of switching branches - git checkout still works, but this is meant to be more intuitive to beginners
There was a problem hiding this comment.
and this creates a new branch too?
There was a problem hiding this comment.
yup, -c for "create"
|
|
||
| ```bash | ||
| $ pip install -r requirements.txt | ||
| $ pip install -e . |
There was a problem hiding this comment.
this includes all the requirements.txt already + gives a local editable install
environment-dev.yml
Outdated
| - recommonmark>=0.4 | ||
| - seaborn>=0.8 | ||
| - sphinx-autobuild>=0.7 | ||
| - sphinx>=1.5 |
There was a problem hiding this comment.
is there a reason to have all these documentation-related dependencies in here or can they be removed? I don't know how the docs are built, but maybe these could live in a separate environment-docs.yml environment
There was a problem hiding this comment.
Yeah, we can move these to a separate env file, but shouldn't just drop them here.
There was a problem hiding this comment.
OK - ended up putting them back an, as they don't take up much extra time anyway, all I've removed now are some nose/unittest-related ones (which shouldn't be necessary any more, given that pytest is used?), and some (bokeh and seaborn) which are only used in a handful of notebooks
MarcoGorelli
changed the title
| channels: | ||
| - conda-forge | ||
| - defaults | ||
| dependencies: | ||
| - python>=3.6 | ||
| - python=3.6 |
There was a problem hiding this comment.
I was getting conflicts when I removed that, made an environment with Python 3.7, and then updated from the file, see #4281
I think we should maybe make three different environment files: environment-dev-py36.yml, environment-dev-py37.yml, and environment-dev-py38.yml
There was a problem hiding this comment.
just tried that, and it works fine (no conflicts!), so that's what I've pushed
There was a problem hiding this comment.
I don't think that's necessary. Can't we pin 38 here instead?
There was a problem hiding this comment.
As in, pin 3.8 here and then run the tests using that?
could do, but I would've thought it would be better to run the tests on the earliest supported version of Python, else someone could accidentally use something modern (e.g. the walrus operator), tests pass, but then pymc3 breaks when people install it using Python3.6. On the other hand, if the tests pass on Python3.6, it's far less likely that they'll break on Python3.8
Like this, the full test suite can be run with environment-dev-py36.yml, the arviz compatitiblity tests with environment-dev-py38.yml, and hopefully most bases will be covered
There was a problem hiding this comment.
Oh, so dev is used to specify the GH action env?
I was thinking this is for people who want to set up a dev environment locally. I suppose it can be both in which case your suggestion of having different envs for different python versions would make sense.
There was a problem hiding this comment.
yup, that's right - check .github/workflows/pytest.yml
- uses: conda-incubator/setup-miniconda@v2
with:
activate-environment: pymc3-dev-py36
channel-priority: strict
environment-file: conda-envs/environment-dev-py36.yml
use-only-tar-bz2: true # IMPORTANT: This needs to be set for caching to work properly!
(on master it's currently environment-dev.yml)
|
This is great, thanks @MarcoGorelli! |
fail_fastin githubactions so that a single failing test doesn't block the other jobsenvironment-dev.yml, so it only contains development packagescloses #4281