Closed maximlt closed 1 year ago
I'm happy with all those proposals, except that I'm not certain of the feasibility of having a separate test environment. Will the linting really succeed fully even if packages needed for the notebook are not installed? My understanding of nbsmoke was that it did depend on such an environment, but if it doesn't (and/or if nbval doesn't), then sure! I definitely do want the notebook evaluation to be in a different environment from the website building, which is crucial for getting the website to build reliably, but that's a separate topic.
Will the linting really succeed fully even if packages needed for the notebook are not installed? My understanding of nbsmoke was that it did depend on such an environment, but if it doesn't (and/or if nbval doesn't), then sure!
Thanks for the good feedback! So as it stands:
nbval
can be run remotely, from an other environment and pointing at the target environment that has the right dependencies. I've implemented that in this PRflake8
has less strict requirements, flake8
can be run from any environment on a code base. Something they advise is to run it from an environment that has the same Python version as the target environment. E.g. if you're running it from a Python 3.6 environment on a code base that uses features added to Python 3.11, it is going to complain about that. In practice I think it won't cause much problem for examples, the notebooks use standard Python features.I definitely do want the notebook evaluation to be in a different environment from the website building, which is crucial for getting the website to build reliably, but that's a separate topic.
Yep this is already implemented, now we'll have to see how well it works across all the projects.
That all sounds great, thanks!
I'm submitting this PR to get feedback on this proposal (cc @jlstevens, @jbednar). Currently all the projects have an additional
test
environment that just addspytest
andnbsmoke
. Projects define thelint
andtest
commands that runpytest --nbsmoke-lint
andpytest --nbsmoke-run
, respectively. These two commands are executed before building a project, a project can't be built if any of them fails.Linting and testing projects is valuable and we should keep doing that. However I started to wonder whether it makes more sense to perform these tests remotely rather than locally. By saying remotely, I mean that instead of each project having to include an additional test environment and two commands, projects would no longer have to do that and the linting and testing would be controlled by code living in
dodo.py
. This is basically what I've implemented in this PR, as a maintainer or contributor of examples you would need to install a managing environment (this will be anyway recommended, for contributors to run all the linters we've added before submitting a PR) and then simply rundoit test:<projectname>
:nbqa
is used to lint the project's notebooks,nbval
is used to test the project's notebooks (I'm using a feature that was just released to target the project's kernel, feature that I believe was implemented by Chris a couple of years ago, thanks!)nbqa
andnbval
being both installed in the managing environment, not in the project's environment.I find this approach interesting as:
nbqa
on a few notebooks and saw many errors reported byflake8
. I'm not sure whatpytest --nbsmoke-lint
was doing really, it looks like it wasn't doing much.But really what I prefer is the removal of the per-project test environment!
Drawbacks include:
test
env spec and commands, so these users would not be affected. Their life would actually improve as they currently get a warning when they run the project, as the lock file still has thetest
spec and anaconda-project doesn't like that much.nbqa
andnbval
may get updates that force us to update some projects (e.g. projects using an old notebook format no longer supported, things like that). So that may have some small maintenance cost. On the other hand I think it actually simplifies maintaining each project.Overall: