search

paris-saclay-cds / ramp-workflow

Toolkit for building predictive workflows on top of pydata (pandas, scikit-learn, pytorch, keras, etc.).
https://paris-saclay-cds.github.io/ramp-docs/
BSD 3-Clause "New" or "Revised" License
68 stars 43 forks source link

Fix CI #321

Closed DimitriPapadopoulos closed 1 year ago

DimitriPapadopoulos commented 1 year ago
DimitriPapadopoulos commented 1 year ago

Almost there, still an issue with docs, despite pinning down Jinja2:

ImportError: cannot import name 'environmentfilter' from 'jinja2'

Any clue? It looks like I am not updating Sphinx or pinning down Jinja2 the right way.

albertcthomas commented 1 year ago

It seems that when doing the sphinx<4.0 install it installs jinja2 3.1.2

Collecting Jinja2>=2.3
  Downloading Jinja2-3.1.2-py3-none-any.whl (133 kB)
albertcthomas commented 1 year ago

It seems that when doing the sphinx<4.0 install it installs jinja2 3.1.2

in fact I don't know why it install sphinx < 4.0, this must be your question then :)

rth commented 1 year ago

Any clue?

Probably CI broke at the time and this was a fix. It we can make it work with the latest sphinx version that would certainly be better.

DimitriPapadopoulos commented 1 year ago

Indeed the question is why it installs sphinx < 4.0. A quick grep comes up with:

setup.py Line 44 in c4faecf

    'docs': ['sphinx>4.1.0', 'sphinx_rtd_theme', 'numpydoc', 'sphinx-click', 'jinja2<3.1.0']

doc/conf.py Line 28 in c4faecf

needs_sphinx = '4.1.0'

It looks like these have no effect at all, something else enforces sphinx<4.0.

albertcthomas commented 1 year ago

Here it asks for sphinx<4.0 in build_doc.sh. This is what is called in CI.

albertcthomas commented 1 year ago

Probably CI broke at the time and this was a fix. It we can make it work with the latest sphinx version that would certainly be better.

Yes, see #280

codecov[bot] commented 1 year ago

Codecov Report

Base: 83.16% // Head: 80.83% // Decreases project coverage by -2.32% :warning:

Coverage data is based on head (759dded) compared to base (4a8d2e5). Patch coverage: 17.43% of modified lines in pull request are covered.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #321 +/- ## ========================================== - Coverage 83.16% 80.83% -2.33% ========================================== Files 137 137 Lines 4752 4947 +195 ========================================== + Hits 3952 3999 +47 - Misses 800 948 +148 ``` | [Impacted Files](https://codecov.io/gh/paris-saclay-cds/ramp-workflow/pull/321?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paris-saclay-cds) | Coverage Δ | | |---|---|---| | [rampwf/externals/tabulate.py](https://codecov.io/gh/paris-saclay-cds/ramp-workflow/pull/321/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paris-saclay-cds#diff-cmFtcHdmL2V4dGVybmFscy90YWJ1bGF0ZS5weQ==) | `15.58% <16.58%> (-0.21%)` | :arrow_down: | | [rampwf/hyperopt/hyperopt.py](https://codecov.io/gh/paris-saclay-cds/ramp-workflow/pull/321/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paris-saclay-cds#diff-cmFtcHdmL2h5cGVyb3B0L2h5cGVyb3B0LnB5) | `98.67% <100.00%> (+0.02%)` | :arrow_up: | | [rampwf/score\_types/clustering\_efficiency.py](https://codecov.io/gh/paris-saclay-cds/ramp-workflow/pull/321/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paris-saclay-cds#diff-cmFtcHdmL3Njb3JlX3R5cGVzL2NsdXN0ZXJpbmdfZWZmaWNpZW5jeS5weQ==) | `100.00% <100.00%> (ø)` | | | [rampwf/hyperopt/cli/hyperopt.py](https://codecov.io/gh/paris-saclay-cds/ramp-workflow/pull/321/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paris-saclay-cds#diff-cmFtcHdmL2h5cGVyb3B0L2NsaS9oeXBlcm9wdC5weQ==) | `0.00% <0.00%> (ø)` | | | [rampwf/hyperopt/tests/test\_hyperparameter.py](https://codecov.io/gh/paris-saclay-cds/ramp-workflow/pull/321/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paris-saclay-cds#diff-cmFtcHdmL2h5cGVyb3B0L3Rlc3RzL3Rlc3RfaHlwZXJwYXJhbWV0ZXIucHk=) | `100.00% <0.00%> (ø)` | | | [rampwf/prediction\_types/combined.py](https://codecov.io/gh/paris-saclay-cds/ramp-workflow/pull/321/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paris-saclay-cds#diff-cmFtcHdmL3ByZWRpY3Rpb25fdHlwZXMvY29tYmluZWQucHk=) | `97.72% <0.00%> (+0.05%)` | :arrow_up: | | [rampwf/tests/test\_kits.py](https://codecov.io/gh/paris-saclay-cds/ramp-workflow/pull/321/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paris-saclay-cds#diff-cmFtcHdmL3Rlc3RzL3Rlc3Rfa2l0cy5weQ==) | `97.18% <0.00%> (+0.08%)` | :arrow_up: | | [rampwf/prediction\_types/detection.py](https://codecov.io/gh/paris-saclay-cds/ramp-workflow/pull/321/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paris-saclay-cds#diff-cmFtcHdmL3ByZWRpY3Rpb25fdHlwZXMvZGV0ZWN0aW9uLnB5) | `85.32% <0.00%> (+0.13%)` | :arrow_up: | | ... and [8 more](https://codecov.io/gh/paris-saclay-cds/ramp-workflow/pull/321/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paris-saclay-cds) | | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paris-saclay-cds). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=paris-saclay-cds)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

DimitriPapadopoulos commented 1 year ago

I will keep sphinx<4.0 for now, and fix CI by pinning down jinja2<3.1.0. Then we can address the upgrade of sphinx in a different merge request.

DimitriPapadopoulos commented 1 year ago

Note that tensoflow and keras are not pinned down any more. A single version for all version of Python doesn't seem to be working.

DimitriPapadopoulos commented 1 year ago

Ready for review.

agramfort commented 1 year ago

thx @DimitriPapadopoulos