maint: merge dependency specifications into setup.cfg #3236

neutrinoceros · 2021-04-29T19:08:59Z

PR Summary

This is a second step forward in trying to solve #3074, following #3078.
Goal: remove as much content as possible from

tests/test_minimal_requirements.txt
tests/test_requirements.txt
andtests/test_prerequirements.txt

and migrate it to setup.cfg
The migration process is error prone and I expect a few iterations will be required.

TODO

This last step can be performed with a conditional similar to

if [ -f test_requirements.txt ] ; then
    # old strategy
else
   # new strategy (replicate what I'm doing in ci_install.sh
fi

Out of scope, followups

Cleanup yaml deps.

Currently minimal build requires pyyaml, but we use pyaml (which itself depends on pyyaml) in the other test envs. Note that both import as yaml... Furtermore, yaml is a soft dependency: part of on_demand_imports, tests use @requires_module("yaml")), but there's actually one place in the code base that we import it without going through on_demand_imports.py :

yt/yt/utilities/mesh_code_generation.py

Line 1 in 16cf684

import yaml

so I'd be inclined to think that it's de-facto already a hard dependency ?

Cleanup pre-existing env targets ?

see

yt/setup.cfg

Line 61 in 16cf684

hub =

apparently the hub target only adds one requirement (girder_hub) and is only used in parts of the command line tool. I'm not sure atm wether this is still used or tested for.

Same goes for the bottle target.

update : according to Kacper the first one is still used.

Remove `tests/windows_conda_requirements.txt`

I introduced this file in the present PR as an intermediate step, though I didn't manage to suppress it completely yet.

Remove `tests/cartopy_requirements.txt`

This is a convenience to mitigate problems with cartopy+shapely distribution. I hope it can be removed in the future, but that will probably require upstream changes regarding packaging.

neutrinoceros · 2021-05-01T15:26:29Z

Since this isn't going to get merged for at least a couple of days, I'll take the opportunity to experiment a little bit further than I originally intended

matthewturk

This looks like an improvement to me. Anyone know if this will impact things like using repo2docker etc?

cphyc

I'm very happy about this! You have my green light once the tests pass

doc/source/developing/testing.rst

cphyc · 2021-05-07T13:00:31Z

tests/ci_install.sh

-      $CYTHON $NUMPY $CARTOPY $H5PY $MATPLOTLIB $SCIPY
+    # windows_conda_requirements.txt is a survivance of test_requirements.txt
+    # keep in sync: setup.cfg
+    while read requirement; do conda install --yes $requirement; done < tests/windows_conda_requirements.txt


Suggested change

while read requirement; do conda install --yes $requirement; done < tests/windows_conda_requirements.txt

conda install --yes --file tests/windows_conda_requirements.txt

I don't think it works unfortunately. To the best of my knowledge, Conda can only install an named environment from a yaml file, and I didn't want to invest any more effort in a temporary band aid

cphyc · 2021-05-07T13:00:55Z

tests/ci_install.sh

-    # unyt so we'll match it here.
-    python -m pip install numpy==1.13.3 cython==0.26.1
-    python -m pip install -r tests/test_minimal_requirements.txt
+    python -m pip install -e .[test,minimal]


So much better!!

cphyc · 2021-05-07T13:05:19Z

@neutrinoceros the errors on Jenkins is because dependencies are still installed by looking for the tests/*_requirements.txt which you deleted. For example it isn't installing nose which is slightly problematic for running the nose test suite.

neutrinoceros

self review to correct typos

tests/ci_install.sh

neutrinoceros · 2021-05-07T13:51:14Z

Thanks @Xarthisius, looks like it worked for the answer test job, and likely for the py38 job as well. The docs and py38_unitests failed however. If there's anything I can do on this size to fix it let me know !

Xarthisius · 2021-05-07T14:08:05Z

Thanks @Xarthisius, looks like it worked for the answer test job, and likely for the py38 job as well. The docs and py38_unitests failed however. If there's anything I can do on this size to fix it let me know !

It's just me being clumsy. It will probably take a couple of iterations until I get it right...

Xarthisius · 2021-05-07T14:08:21Z

@yt-fido test this please

Xarthisius · 2021-05-07T14:09:47Z

BTW there are extra dependencies for docs that could be added to the setup:

pip install pyregion Sphinx==3.1.2 alabaster sphinx-bootstrap-theme sphinx-rtd-theme RunNotebook
pip install -U nbconvert==5.6.1

I don't remember specific bugs that resulted in pinning Sphinx and nbconvert, but they're there for a reason I'm sure.

neutrinoceros · 2021-05-07T14:23:20Z

It's just me being clumsy. It will probably take a couple of iterations until I get it right...

It took a lot more iterations to me, I get it :)

BTW there are extra dependencies for docs that could be added to the setup

Good to know, I'll make sure to include this (back in a moment).

neutrinoceros · 2021-05-07T15:22:08Z

@Xarthisius does this work for you ?
69a262b

Xarthisius · 2021-05-11T13:56:00Z

@Xarthisius does this work for you ?
69a262b

LGTM. Thanks!

Xarthisius · 2021-05-12T14:46:27Z

@neutrinoceros ~~I'm doing python -m pip install -e '.[test,full,docs]' and it doesn't install sphinx. Any hints?~~
Maybe, because it's doc not docs...

neutrinoceros · 2021-05-12T14:49:54Z

@neutrinoceros I'm doing python -m pip install -e '.[test,full,docs]' and it doesn't install sphinx. Any hints?
Maybe, because it's doc not docs...

ah well, that's a good reason :)
I'm surprised that it doesn't give better hints on this kind of error though.

neutrinoceros · 2021-05-14T07:43:48Z

I really don't get the what's happening in the one test that still breaks on Jenkins.
@Xarthisius can you show me the scripts you're using there to install the deps, both old and new ?

Also I think your setup for the docs job is likely ok now, but we ran into a timeout two times in a row... (third time's the charm ?)

@yt-fido test this please

neutrinoceros · 2021-05-14T09:18:56Z

Quick note that although the remaining failure happens consistently, the quantitative error that's produced fluctuates.
update: I can't reproduce this locally for now
The failing test (yt/frontends/athena_pp/tests/test_outputs.py::test_disk) may be failing because some values are left uninitialised somehow (hence the fluctuations ?), but it's not clear to me why it would only show up now. Maybe I'm effectively changing the version of numpy that gets installed and that version somehow tends to produce larger values from uninitiated memory ?

neutrinoceros · 2021-05-22T15:34:08Z

@yt-fido test this please

…into setup.cfg

Xarthisius · 2021-06-11T14:39:08Z

@yt-fido test this please

neutrinoceros · 2021-06-11T14:54:35Z

@Xarthisius I just performed some diffs between a reference run and the latest one here for the failing docs job on Jenkins and an important difference is that on this branch, we apparently install yt twice, as the log contains

python -m pip install '.[test,full,doc]'
...
python -m pip install -e '.[test,full,doc]'

while in the ref run, it looks to me that yt itself is never installed but is only importable because we run

python setup.py build_ext -i -f

and every test is run from the top level of the repo.
I don't know if this sole difference can account for more than an hour of additional overhead though, but there seems to be room for optimisation there (install just once, and probably not in editable mode).
Any chance you already found anything more interesting/important/suspicious ? :)

Xarthisius · 2021-06-11T15:03:55Z

Doc build failed due to timeout. It happens from time to time when sphinx subthread dies for some reason. The build is kinda brittle this way. I'd give another chance (which I did a while ago).

neutrinoceros · 2021-06-11T15:50:58Z

oh I see now that it's hanging again against the same volume rendering recipe that it did earlier today. The timeout already seemed to happen consistently on this branch.

Xarthisius · 2021-06-11T15:52:10Z

The errors I'm seeing in the log (that are causing the timeout):

Traceback (most recent call last):
  File "temp.py", line 3, in <module>
    ds = yt.load("gadget_cosmology_plus/snap_N128L16_132.hdf5")
  File "/var/jenkins_home/workspace/yt_docs_new/yt/loaders.py", line 81, in load
    fn = str(lookup_on_disk_data(fn))
  File "/var/jenkins_home/workspace/yt_docs_new/yt/sample_data/api.py", line 168, in lookup_on_disk_data
    raise FileNotFoundError(err_msg)
FileNotFoundError: No such file or directory: 'gadget_cosmology_plus/snap_N128L16_132.hdf5'.

Traceback (most recent call last):
  File "temp.py", line 10, in <module>
    profile = yt.create_profile(
  File "/var/jenkins_home/workspace/yt_docs_new/yt/data_objects/profiles.py", line 1494, in create_profile
    obj.set_field_unit(field, unit)
  File "/var/jenkins_home/workspace/yt_docs_new/yt/data_objects/profiles.py", line 121, in set_field_unit
    fd = self.field_map[field]
KeyError: ('gas', 'cell_mass')

Xarthisius · 2021-06-11T15:53:04Z

How about rebasing onto main if you haven't already?

neutrinoceros · 2021-06-11T15:56:07Z

How about rebasing onto main if you haven't already?

that what I did a few hours ago

neutrinoceros · 2021-06-11T15:58:17Z

This errors you mentioned also happen on a successful run. See for instance: https://tests.yt-project.org/job/yt_docs_new/1508/consoleFull

Xarthisius · 2021-06-11T16:30:23Z

The reason it fails is this:

python: LineString.cpp:121: const geos::geom::CoordinateSequence* geos::geom::LineString::getCoordinatesRO() const: Assertion `0 != points.get()' failed.                                      

Exception occurred:
  File "/var/jenkins_home/shiningpanda/jobs/c9e5cb26/virtualenvs/d41d8cd9/lib/python3.8/site-packages/nbconvert/preprocessors/execute.py", line 510, in _check_alive
    raise DeadKernelError("Kernel died")
nbconvert.preprocessors.execute.DeadKernelError: Kernel died

we get that error if shapely is not compiled with the version of GEOS library present on the testing box. In requirements.txt it's taken care of by --no-binary flag. We need an equivalent for setup.py provided it exists...

neutrinoceros · 2021-06-11T18:59:18Z

@yt-fido test this please

neutrinoceros · 2021-06-11T20:17:58Z

Hurray ! Many thanks to @Xarthisius for your help with this one !

neutrinoceros added the infrastructure Related to CI, versioning, websites, organizational issues, etc label Apr 29, 2021

neutrinoceros force-pushed the cleanup_test_requirement_files branch 3 times, most recently from 15e972b to 0eace7e Compare April 30, 2021 06:24

neutrinoceros marked this pull request as ready for review April 30, 2021 06:24

neutrinoceros changed the title ~~Cleanup test requirement files~~ maint: merge dependency specifications into setup.cfg Apr 30, 2021

neutrinoceros mentioned this pull request Apr 30, 2021

Out of date requirements (unused optional deps ?) #3237

Closed

neutrinoceros force-pushed the cleanup_test_requirement_files branch from 0eace7e to fcb095a Compare April 30, 2021 12:05

neutrinoceros marked this pull request as draft May 1, 2021 15:28

neutrinoceros force-pushed the cleanup_test_requirement_files branch 2 times, most recently from c1c3d4a to 5b033c6 Compare May 1, 2021 16:48

neutrinoceros marked this pull request as ready for review May 1, 2021 17:00

This was referenced May 2, 2021

CI: add the pytest-randomly pytest plugin to test env #3245

Closed

Update Docs for Pytest #3131

Merged

matthewturk approved these changes May 6, 2021

View reviewed changes

cphyc approved these changes May 7, 2021

View reviewed changes

neutrinoceros commented May 7, 2021

View reviewed changes

tests/ci_install.sh Outdated Show resolved Hide resolved

tests/ci_install.sh Outdated Show resolved Hide resolved

This was referenced May 9, 2021

exp: refactor the on demand import module #3246

Closed

maint: pin numpy maximal version #3172

Closed

neutrinoceros mentioned this pull request May 26, 2021

Update all docs to reflect yt4 changes and fix breakages #3281

Closed

15 tasks

neutrinoceros force-pushed the cleanup_test_requirement_files branch from da8735f to e99429c Compare May 28, 2021 10:08

neutrinoceros mentioned this pull request Jun 2, 2021

CI: failures on the main branch (?) #3319

Closed

neutrinoceros force-pushed the cleanup_test_requirement_files branch from e99429c to ec04cdb Compare June 6, 2021 08:44

neutrinoceros added 2 commits June 11, 2021 11:25

maint: cleanup test requirement files. Migrate most of their content …

8ec9750

…into setup.cfg

include doc env in setup.cfg

c30980f

neutrinoceros force-pushed the cleanup_test_requirement_files branch from ec04cdb to c30980f Compare June 11, 2021 09:25

Xarthisius force-pushed the cleanup_test_requirement_files branch 2 times, most recently from f39634e to 1b4411d Compare June 11, 2021 16:56

maint: put cartopy specs back into a requirement file

7613a03

neutrinoceros force-pushed the cleanup_test_requirement_files branch from 1b4411d to 7613a03 Compare June 11, 2021 17:32

Xarthisius approved these changes Jun 11, 2021

View reviewed changes

Xarthisius merged commit 1922654 into yt-project:main Jun 11, 2021

neutrinoceros deleted the cleanup_test_requirement_files branch June 11, 2021 20:18

This was referenced Jun 12, 2021

Support for Python versions for upcoming 4.0 release #3070

Closed

Technical debt with deps specifications #3074

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

maint: merge dependency specifications into setup.cfg #3236

maint: merge dependency specifications into setup.cfg #3236

neutrinoceros commented Apr 29, 2021 •

edited

Loading

neutrinoceros commented May 1, 2021

matthewturk left a comment

cphyc left a comment

cphyc May 7, 2021

neutrinoceros May 7, 2021

cphyc May 7, 2021

cphyc commented May 7, 2021

neutrinoceros left a comment

neutrinoceros commented May 7, 2021

Xarthisius commented May 7, 2021

Xarthisius commented May 7, 2021

Xarthisius commented May 7, 2021

neutrinoceros commented May 7, 2021

neutrinoceros commented May 7, 2021

Xarthisius commented May 11, 2021

Xarthisius commented May 12, 2021 •

edited

Loading

neutrinoceros commented May 12, 2021

neutrinoceros commented May 14, 2021

neutrinoceros commented May 14, 2021 •

edited

Loading

neutrinoceros commented May 22, 2021

Xarthisius commented Jun 11, 2021

neutrinoceros commented Jun 11, 2021 •

edited

Loading

Xarthisius commented Jun 11, 2021 •

edited

Loading

neutrinoceros commented Jun 11, 2021

Xarthisius commented Jun 11, 2021

Xarthisius commented Jun 11, 2021

neutrinoceros commented Jun 11, 2021

neutrinoceros commented Jun 11, 2021

Xarthisius commented Jun 11, 2021

neutrinoceros commented Jun 11, 2021

neutrinoceros commented Jun 11, 2021

	while read requirement; do conda install --yes $requirement; done < tests/windows_conda_requirements.txt
	conda install --yes --file tests/windows_conda_requirements.txt

maint: merge dependency specifications into setup.cfg #3236

maint: merge dependency specifications into setup.cfg #3236

Conversation

neutrinoceros commented Apr 29, 2021 • edited Loading

PR Summary

Out of scope, followups

Cleanup yaml deps.

Cleanup pre-existing env targets ?

Remove tests/windows_conda_requirements.txt

Remove tests/cartopy_requirements.txt

neutrinoceros commented May 1, 2021

matthewturk left a comment

Choose a reason for hiding this comment

cphyc left a comment

Choose a reason for hiding this comment

cphyc May 7, 2021

Choose a reason for hiding this comment

neutrinoceros May 7, 2021

Choose a reason for hiding this comment

cphyc May 7, 2021

Choose a reason for hiding this comment

cphyc commented May 7, 2021

neutrinoceros left a comment

Choose a reason for hiding this comment

neutrinoceros commented May 7, 2021

Xarthisius commented May 7, 2021

Xarthisius commented May 7, 2021

Xarthisius commented May 7, 2021

neutrinoceros commented May 7, 2021

neutrinoceros commented May 7, 2021

Xarthisius commented May 11, 2021

Xarthisius commented May 12, 2021 • edited Loading

neutrinoceros commented May 12, 2021

neutrinoceros commented May 14, 2021

neutrinoceros commented May 14, 2021 • edited Loading

neutrinoceros commented May 22, 2021

Xarthisius commented Jun 11, 2021

neutrinoceros commented Jun 11, 2021 • edited Loading

Xarthisius commented Jun 11, 2021 • edited Loading

neutrinoceros commented Jun 11, 2021

Xarthisius commented Jun 11, 2021

Xarthisius commented Jun 11, 2021

neutrinoceros commented Jun 11, 2021

neutrinoceros commented Jun 11, 2021

Xarthisius commented Jun 11, 2021

neutrinoceros commented Jun 11, 2021

neutrinoceros commented Jun 11, 2021

neutrinoceros commented Apr 29, 2021 •

edited

Loading

Remove `tests/windows_conda_requirements.txt`

Remove `tests/cartopy_requirements.txt`

Xarthisius commented May 12, 2021 •

edited

Loading

neutrinoceros commented May 14, 2021 •

edited

Loading

neutrinoceros commented Jun 11, 2021 •

edited

Loading

Xarthisius commented Jun 11, 2021 •

edited

Loading