[Python] Add requirements.txt and test #5408

stwunsch · 2020-04-21T05:15:50Z

Proposal to add a requirements.txt file and a corresponding test checking the dependencies also by the version requirement.

The test throws an error for each dependency seperately due to the SubTest. A single exception looks like this:

3: pkg_resources.VersionConflict: (xgboost 0.80 (/home/stefan/.local/lib/python3.8/site-packages), Requirement.parse('xgboost>=0.81'))

@eguiraud @Axel-Naumann @etejedor @oshadura Ping :)

stwunsch · 2020-04-21T05:16:32Z

@oshadura How can I get the path to the requirements.txt in the test? ROOTSYS does not really help because it points to the build dir, I would need the path to the source dir.

vepadulano · 2020-04-21T07:52:44Z

@oshadura How can I get the path to the requirements.txt in the test? ROOTSYS does not really help because it points to the build dir, I would need the path to the source dir.

Supposing the test is under "path/to/root/bindings/pyroot_experimental/pythonizations/test/" you could do:

import os
abspath = os.path.abspath(__file__) # path/to/root/bindings/pyroot_experimental/pythonizations/test/test.py
reqpath = os.path.join(abspath.partition("bindings/")[0], "requirements.txt")  # path/to/root/requirements.txt

with open(reqpath) as f:
...

Though I wonder whether the requirements.txt file would be better suited under the bindings folder since it's python related?

stwunsch · 2020-04-21T08:06:56Z

I didn't want to do too much path magic, too prone to break ;) The location of the requirements.txt file is also a good question. @axel @oshadura ? People would expect it in the root folder though.

vepadulano · 2020-04-21T08:17:02Z

I get what you mean, but in this case I don't think the location of the bindings/ folder will change anytime relative to the home root folder right? Otherwise, a new ROOTHOME variable could be set along with ROOTSYS ?

stwunsch · 2020-04-21T08:21:29Z

Do we run the tests always from the source directory? I'm not 100% sure about this.

vepadulano · 2020-04-21T08:27:33Z

I believe that os.path.abspath always returns the absolute path starting from the / folder of the machine, irrespective of the folder from where the test is running. At least on linux, not sure about windows

vepadulano · 2020-04-22T08:29:12Z

There is also root-config --srcdir that actually returns the path to the main root directory, it could be called with subprocess in python

stwunsch · 2020-04-22T09:53:44Z

Nice! That's actually a quite nice solution, and could be viable for the tests. Just hope you don't move the source dir ;) Could be a start!

eguiraud · 2020-04-22T11:33:50Z

Hi,
isn't it enough (and more similar to what users would experience) to run pip check? It could even be done at ROOT configuration time if -Dpython=ON

stwunsch · 2020-04-22T13:25:10Z

@eguiraud Good question! I don't know whether we want to enforce this a configuration time. Would make sense, tbh, though this would enforce much more stricter requirements than the ones we have now.

oshadura · 2020-04-23T08:26:51Z

I didn't want to do too much path magic, too prone to break ;) The location of the requirements.txt file is also a good question. @axel @oshadura ? People would expect it in the root folder though.

About location, totally make sense!

oshadura · 2020-04-23T08:27:49Z

Hi,
isn't it enough (and more similar to what users would experience) to run pip check? It could even be done at ROOT configuration time if -Dpython=ON

We don't have this option anymore :) Now it is "pyroot"...

oshadura · 2020-04-23T08:28:56Z

Why not to add in "dev" option?

stwunsch · 2020-04-23T08:35:50Z

So I think what we agreed on on Mattermost: We want the thingy, but no checks at configure time (because it's not a build time dependency). We still have to iterate how exactly we want to check at test time the dependencies, but I think at a first shot, the current impl is good enough. Feel free to argue :)

stwunsch · 2020-04-23T09:33:15Z

@phsft-bot build

phsft-bot · 2020-04-23T09:33:25Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos7-multicore/default, ROOT-fedora29/python3, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1015/cxx17, windows10/cxx14
How to customize builds

phsft-bot · 2020-04-23T09:35:37Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos7-multicore/default, ROOT-fedora29/python3, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1015/cxx17, windows10/cxx14
How to customize builds

phsft-bot · 2020-04-23T09:54:54Z

Build failed on ROOT-fedora31/noimt.
Running on root-fedora-31-1.cern.ch:/build/workspace/root-pullrequests-build
See cdash.
See console output.

Failing tests:

projectroot.bindings.pyroot_experimental.pythonizations.test.pyunittests_pyroot_dependency_versions

phsft-bot · 2020-04-23T10:00:45Z

Build failed on mac1015/cxx17.
Running on macphsft18.dyndns.cern.ch:/Users/sftnight/build/jenkins/workspace/root-pullrequests-build
See cdash.
See console output.

Failing tests:

projectroot.bindings.pyroot_experimental.pythonizations.test.pyunittests_pyroot_dependency_versions

stwunsch · 2020-07-07T07:07:29Z

But it's a good question, because we do not depend on pandas like on jupyter or numba. we just support to push data to it and test a specific interface of pandas.

phsft-bot · 2020-07-07T07:20:27Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos7-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac1015/cxx17, windows10/cxx14
How to customize builds

phsft-bot · 2020-07-07T08:01:39Z

Build failed on ROOT-debian10-i386/cxx14.
Running on pcepsft10.dyndns.cern.ch:/build/workspace/root-pullrequests-build
See cdash.
See console output.

Failing tests:

projectroot.bindings.pyroot.pythonizations.test.pyunittests_pyroot_dependency_versions

stwunsch · 2020-07-07T08:52:40Z

Here an example from the failure above how the diagnostics look like:

Full path to requirements.txt: /home/sftnight/build/workspace/root-pullrequests-build/root/requirements.txt
Details about not matched dependencies:
 - The 'numba>=0.47.0' distribution was not found and is required by the application

stwunsch · 2020-07-07T09:21:13Z

We should merge root-project/rootspi#81 first and retry the PR build (afaik rootspi is getting fetched again on each build).

stwunsch · 2020-07-07T10:05:47Z

@phsft-bot build

phsft-bot · 2020-07-07T10:05:55Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos7-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac1015/cxx17, windows10/cxx14
How to customize builds

phsft-bot · 2020-07-07T11:56:01Z

Build failed on mac1014/python3.
Running on macitois21.cern.ch:/build/jenkins/workspace/root-pullrequests-build
See cdash.
See console output.

Failing tests:

And 24 more

stwunsch · 2020-07-07T13:18:01Z

@eguiraud I would say the CI is happy ;)

stwunsch · 2020-07-07T13:19:24Z

You think it's good to go like this? It does not have yet all dependencies, e.g., the ones for TMVA experimental are missing. Though we should could have with this a first run through the nightlies and fix failures in our infrastructure (missing packages, version mismatches, ...).

stwunsch · 2020-07-07T13:19:53Z

I think there should be little failures since we switch already long ago to the "just fail the test" mode. But you never know :)

eguiraud · 2020-07-07T14:22:25Z

I think there should be little failures since we switch already long ago to the "just fail the test" mode. But you never know :)

There is a ton of tutorials that would fail that currently do not run, see #5938 . But I don't think this PR interferes with that, right @hageboeck ?

hageboeck · 2020-07-07T15:15:07Z

I think there should be little failures since we switch already long ago to the "just fail the test" mode. But you never know :)

There is a ton of tutorials that would fail that currently do not run, see #5938 . But I don't think this PR interferes with that, right @hageboeck ?

They are kind of doing the same thing, but #5938 is dumb. It only checks if it can import <package> at all, and then runs the tutorial or fails all that need e.g. numpy. It doesn't care for versions. I guess I can rewrite the ${PYTHON_EXECUTABLE_DevMain} -c "import <package>" from the other PR to simply use the test from this PR.

For this, however, it would be nice if you could ask for specific packages. So the thing would look up numba in the requirements, and test if the correct version is installed to run a tutorial or test.

This way, we would also get a cross-check that dependencies of tests/tutorials are actually in requirements.txt.

eguiraud

Alright, I think this can go in and together with the labels that #5938 adds it should address what we agreed upon at yesterday's meeting

hageboeck · 2020-07-07T15:21:54Z

I edited my previous comment, maybe check again.

hageboeck · 2020-07-07T15:24:56Z

Also, we might be missing numpy, keras, xgboost in the requirements.

hageboeck · 2020-07-07T15:26:00Z

And pandas.

eguiraud · 2020-07-07T15:36:51Z

This PR solves the problem of documenting our python requirements and adding a test that checks if we satisfy them at runtime.

#5938 solves three main problems (plus a couple smaller things): a bug in an RDF tutorial, a bug in the vetoing of tutorials, and it adds a nice-to-have, namely having a single test fail that's clearly called "test-if-numba-is-there" rather than having all numba tests red.

If what I said above is in the ballpark of correct, there is no conflict with #5938. so I still think this PR could go in modulo the missing requirements -- consider my approval conditional to agreeing on what should go in requirements.txt (i'll stay out of the discussion about whether pandas should be in there or not :D)

stwunsch · 2020-07-07T15:51:20Z

Alrighty, I'll add sklearn, numpy, xgboost and keras one by one and check against the PR builds the status on the nodes. This will cause havoc for a few iterations 😬

hageboeck · 2020-07-07T16:05:55Z

Alrighty, I'll add sklearn, numpy, xgboost and keras one by one and check against the PR builds the status on the nodes. This will cause havoc for a few iterations 😬

I guess that's what Philippe meant. You can merge as-is, and I'll adapt the other PR accordingly. Let's maybe agree that the ML things are not hard requirements.

phsft-bot · 2020-07-07T17:41:39Z

Starting build on ROOT-debian10-i386/cxx14, ROOT-performance-centos7-multicore/default, ROOT-fedora30/cxx14, ROOT-fedora31/noimt, ROOT-ubuntu16/nortcxxmod, mac1014/python3, mac1015/cxx17, windows10/cxx14
How to customize builds

stwunsch · 2020-07-07T17:43:27Z

Alrighty, I'll add sklearn, numpy, xgboost and keras one by one and check against the PR builds the status on the nodes. This will cause havoc for a few iterations grimacing

I guess that's what Philippe meant. You can merge as-is, and I'll adapt the other PR accordingly. Let's maybe agree that the ML things are not hard requirements.

For me it means that we have actually no idea whether some things work or not ;) I've added numpy and that's good for now. I think it's overkill to add the TMVA deps now, because I also don't know exactly what the required versions are.

stwunsch · 2020-07-07T17:47:15Z

There's now https://sft.its.cern.ch/jira/browse/ROOT-10909 to track this issue.

phsft-bot · 2020-07-07T17:51:00Z

Build failed on ROOT-fedora31/noimt.
Running on root-fedora-31-2.cern.ch:/home/sftnight/build/workspace/root-pullrequests-build
See cdash.
See console output.

Failing tests:

projectroot.roottest.root.dataframe.roottest_root_dataframe_ctors

phsft-bot · 2020-07-07T18:37:33Z

Build failed on mac1014/python3.
Running on macitois21.cern.ch:/build/jenkins/workspace/root-pullrequests-build
See cdash.
See console output.

Failing tests:

And 3 more

stwunsch added the improvement label Apr 21, 2020

stwunsch requested review from Axel-Naumann, etejedor and oshadura as code owners April 21, 2020 05:15

stwunsch self-assigned this Apr 21, 2020

stwunsch requested a review from eguiraud April 22, 2020 07:00

stwunsch force-pushed the test-pydeps branch from 696d323 to 76bb9d0 Compare April 23, 2020 09:31

stwunsch changed the title ~~[NFC][Python] Add requirements.txt and test~~ [Python] Add requirements.txt and test Apr 23, 2020

stwunsch added new feature needs review and removed improvement labels Apr 23, 2020

[Python] Respect vetoed features in the dependency tests

5d6ced8

eguiraud approved these changes Jul 7, 2020

View reviewed changes

[Python] Add numpy to requirements.txt

c898a10

stwunsch removed the needs review label Jul 7, 2020

stwunsch merged commit 366d946 into root-project:master Jul 7, 2020

vepadulano mentioned this pull request Feb 4, 2024

[ROOT-10909] Add TMVA python dependencies to the requirements.txt #14553

Closed

[Python] Add requirements.txt and test #5408

[Python] Add requirements.txt and test #5408

Conversation

stwunsch commented Apr 21, 2020 • edited Loading

stwunsch commented Apr 21, 2020

vepadulano commented Apr 21, 2020 • edited Loading

stwunsch commented Apr 21, 2020

vepadulano commented Apr 21, 2020

stwunsch commented Apr 21, 2020

vepadulano commented Apr 21, 2020

vepadulano commented Apr 22, 2020

stwunsch commented Apr 22, 2020

eguiraud commented Apr 22, 2020 • edited Loading

stwunsch commented Apr 22, 2020

oshadura commented Apr 23, 2020

oshadura commented Apr 23, 2020

oshadura commented Apr 23, 2020

stwunsch commented Apr 23, 2020

stwunsch commented Apr 23, 2020

phsft-bot commented Apr 23, 2020

phsft-bot commented Apr 23, 2020

phsft-bot commented Apr 23, 2020

Failing tests:

phsft-bot commented Apr 23, 2020

Failing tests:

stwunsch commented Jul 7, 2020

phsft-bot commented Jul 7, 2020

phsft-bot commented Jul 7, 2020

Failing tests:

stwunsch commented Jul 7, 2020

stwunsch commented Jul 7, 2020

stwunsch commented Jul 7, 2020

phsft-bot commented Jul 7, 2020

phsft-bot commented Jul 7, 2020

Failing tests:

stwunsch commented Jul 7, 2020

stwunsch commented Jul 7, 2020

stwunsch commented Jul 7, 2020

eguiraud commented Jul 7, 2020

hageboeck commented Jul 7, 2020 • edited Loading

eguiraud left a comment

Choose a reason for hiding this comment

hageboeck commented Jul 7, 2020

hageboeck commented Jul 7, 2020

hageboeck commented Jul 7, 2020

eguiraud commented Jul 7, 2020

stwunsch commented Jul 7, 2020 • edited Loading

hageboeck commented Jul 7, 2020

phsft-bot commented Jul 7, 2020

stwunsch commented Jul 7, 2020

stwunsch commented Jul 7, 2020 • edited Loading

phsft-bot commented Jul 7, 2020

Failing tests:

phsft-bot commented Jul 7, 2020

Failing tests:

stwunsch commented Apr 21, 2020 •

edited

Loading

vepadulano commented Apr 21, 2020 •

edited

Loading

eguiraud commented Apr 22, 2020 •

edited

Loading

hageboeck commented Jul 7, 2020 •

edited

Loading

stwunsch commented Jul 7, 2020 •

edited

Loading

stwunsch commented Jul 7, 2020 •

edited

Loading