Skip to content

install python module to site-dir (ROOT-3316)#73

Closed
xantares wants to merge 1 commit intoroot-project:masterfrom
xantares:bugfix/ROOT-3316
Closed

install python module to site-dir (ROOT-3316)#73
xantares wants to merge 1 commit intoroot-project:masterfrom
xantares:bugfix/ROOT-3316

Conversation

@xantares
Copy link
Copy Markdown

This patch installs python modules to python site-dir standard location (see some doc here:https://docs.python.org/2/library/site.html), see https://sft.its.cern.ch/jira/browse/ROOT-3316

It avoids to have to set PYTHONPATH when installing to a system folder /usr or /usr/local, and even the user site-dir ~/.local

Packaging may have to be reworked though (https://www.debian.org/doc/packaging-manuals/python-policy/ch-python.html#s-paths)

@xantares
Copy link
Copy Markdown
Author

xantares commented Sep 3, 2015

hi @karies, @peremato, someone up for a review ?

@Axel-Naumann
Copy link
Copy Markdown
Member

@wlav should have a look as he seemed to have good reasons against such a change on https://sft.its.cern.ch/jira/browse/ROOT-3316.

@wlav
Copy link
Copy Markdown
Contributor

wlav commented Sep 3, 2015

Hi,

yes, my objections are all documented in that jira report. I strongly
vote 'no'. If anyone wants to go ahead anyway, then sign a contract in
blood that you'll maintain it and help confused users from here on out.

Best regards,
Wim

On Thursday 2015-09-03 05:08, Axel Naumann wrote:

Date: Thu, 03 Sep 2015 05:08:04 -0700
From: Axel Naumann notifications@github.com
Reply-To: root-mirror/root
<reply+00613b66d70f8ce7beaed1a94ac44c0adf0329085ffc340492cf0000000111fffa2
492a169ce05d53aeb@reply.github.com>
To: root-mirror/root root@noreply.github.com
Cc: wlav WLavrijsen@lbl.gov
Subject: Re: [root] install python module to site-dir (ROOT-3316) (#73)

@wlav should have a look as seemed to have good reasons against such a change on https://sft.its.cern.ch/jira/browse/ROOT-3316.


Reply to this email directly or view it on GitHub:
#73 (comment)

WLavrijsen@lbl.gov -- +1 (510) 486 6411 -- www.lavrijsen.net

@xantares
Copy link
Copy Markdown
Author

xantares commented Sep 3, 2015

hello @wlav,

I was confused to have to set PYTHONPATH in the first place, like Andy was :)

Forgive me but your objections do not seem rock-solid:

  • '"distutils.sysconfig" is far from robust as claimed' : afaik it works very well from 2.6-2.7, 3.1,3.2, 3.3 to 3.4
  • "package as a whole gets split, which risks that one part gets updated whereas another doesn't": how would that be possible for users ? a decent package manager should handle that well

regards

@wlav
Copy link
Copy Markdown
Contributor

wlav commented Sep 3, 2015

Hi,

On Thursday 2015-09-03 10:56, xantares wrote:

Forgive me but your objections do not seem rock-solid:

  • '"distutils.sysconfig" is far from robust as claimed' : afaik it
    works very well from 2.6-2.7, 3.1,3.2, 3.3 to 3.4

sure, on Linux. Did you try all main platforms? (And do add at least p2.5,
as that for sure is still in use.)

  • "package as a whole gets split, which risks that one part gets updated
    whereas another doesn't": how would that be possible for users ? a
    decent package manager should handle that well

A package manager can install wherever it wants and can make its own
adjustment (simply copy over the two files, or put symlinks); this will
affect users building from source. I don't worry about package managers,
only about the latter users. As to 'how': the common case mixing
installations (e.g. from packagers and building from source), versions
(different pythons), system upgrades (that wipe out site-packages), or
simply a user doing 'rm -rf' for only half the installation. Users are
very, very inventive in creating trouble. :P None of that if all of ROOT
is kept in a single directory.

So the only upside there seems to be is removing of this teeny-weeny
inconvenience of having to setup PYTHONPATH if you install in /usr/local.
But A) most from-source builders don't do that, they use bin/thisroot.sh,
which sets up PYTHONPATH; and B) installing in /usr/local is not
recommended to begin with, b/c of the same problem with remnants.

Debugging a setup problem is very time consuming and frustrating: the
error messages are spurious and only occur on the user's machine to which
I have no access.

In sum, I see no upside to speak of, but do see enormous downsides. And
if you really believe otherwise: sign that blood contract.

Best regards,

Wim

WLavrijsen@lbl.gov -- +1 (510) 486 6411 -- www.lavrijsen.net

@xantares
Copy link
Copy Markdown
Author

xantares commented Sep 9, 2015

@wlav
"So the only upside there seems to be is removing of this teeny-weeny inconvenience of having to setup PYTHONPATH if you install in /usr/local. But A) most from-source builders don't do that, they use bin/thisroot.sh, which sets up PYTHONPATH; and B) installing in /usr/local is not recommended to begin with, b/c of the same problem with remnants."

A) it's also valid when installing as user to ~/.local (like when you install a python module with --user)
B) of course, users shouldn't install anything to /usr[/local] without using package management, but it will simplify packaging too by not having to install an ugly script somewhere that sets PYTHONPATH

As for the downsides, I think this solution is just simpler (not mentioning cleaner), maybe this will get you less bug reports.

@wlav
Copy link
Copy Markdown
Contributor

wlav commented Sep 10, 2015

Hi,

very last time and then I give up ...

B) of course, users shouldn't install anything to /usr[/local] without
using package management

But they do.

but it will simplify packaging too by not having to install an ugly
script somewhere that sets PYTHONPATH

Ugly scripts that are seldom seen. I care more about humans than computers.
Additionally, the removal scripts can be simpler, as the package manager
knows where it put what, rather than the package stuffing portions in
different places, so I'd say its a wash.

As for the downsides, I think this solution is just simpler (not
mentioning cleaner), maybe this will get you less bug reports.

Straight of the bat, the whole nightly build system would roll over and
all developers that use cmake (not me, luckily) would have to change their
workflow. Why? B/c most of these setups are 1 python installation and
multiple ROOT installations, or different users for both so no access
rights to site packages. Add that packagers for LCG experiments now need
to know that ROOT puts stuff in $ROOTSYS and in $PYTHONHOME and fix what
they pick up from where. For that matter, I think there will be far more
'ugly scripts' being created here to work around the mess this would add,
that I dare claim that even that argument of yours holds no water.

Like I said, this is my last word on it. It's not worth my time if you
refuse to look beyond that one use case.

Best regards,

Wim

WLavrijsen@lbl.gov -- +1 (510) 486 6411 -- www.lavrijsen.net

guitargeek added a commit to guitargeek/root that referenced this pull request Mar 7, 2026
For small unit tests like the RooFit pythonization tests, the overhead
of starting the Python interpreter is dominant. That means the tests
should not be too granular. I think testing the RooFit pythonizations
with separate Python executions at the level of the different RooFit
classes was indeed too granular. Therefore, this commit suggests to have
a single RooFit pythonization unit test.

Running the unit tests before (single thread):
```txt
Test project /home/rembserj/code/root/root_build/bindings/pyroot/pythonizations/test
      Start 64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabscollection
 1/14 Test root-project#64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabscollection .......   Passed    1.09 sec
      Start 65: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabspdf-fitto
 2/14 Test root-project#65: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabspdf-fitto ........   Passed    1.13 sec
      Start 66: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabsreal-ploton
 3/14 Test root-project#66: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabsreal-ploton ......   Passed    1.14 sec
      Start 67: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooarglist
 4/14 Test root-project#67: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooarglist .............   Passed    0.93 sec
      Start 68: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roocmdarg
 5/14 Test root-project#68: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roocmdarg ..............   Passed    0.95 sec
      Start 69: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-numpy
 6/14 Test root-project#69: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-numpy ......   Passed    1.32 sec
      Start 70: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-ploton
 7/14 Test root-project#70: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-ploton .....   Passed    1.09 sec
      Start 71: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset
 8/14 Test root-project#71: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset .............   Passed    1.03 sec
      Start 72: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset-numpy
 9/14 Test root-project#72: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset-numpy .......   Passed    1.70 sec
      Start 73: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooglobalfunc
10/14 Test root-project#73: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooglobalfunc ..........   Passed    1.21 sec
      Start 74: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roojsonfactorywstool
11/14 Test root-project#74: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roojsonfactorywstool ...   Passed    1.40 sec
      Start 75: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roolinkedlist
12/14 Test root-project#75: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roolinkedlist ..........   Passed    0.87 sec
      Start 76: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roosimultaneous
13/14 Test root-project#76: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roosimultaneous ........   Passed    1.09 sec
      Start 77: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooworkspace
14/14 Test root-project#77: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooworkspace ...........   Passed    1.21 sec

100% tests passed, 0 tests failed out of 14

Label Time Summary:
python                 =  16.16 sec*proc (14 tests)
python_runtime_deps    =   3.02 sec*proc (2 tests)

Total Test time (real) =  16.17 sec
```

With this commit:
```txt
Test project /home/rembserj/code/root/root_build/bindings/pyroot/pythonizations/test
    Start 64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit
1/1 Test root-project#64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit ...   Passed    3.27 sec

100% tests passed, 0 tests failed out of 1

Label Time Summary:
python    =   3.27 sec*proc (1 test)

Total Test time (real) =   3.27 sec
```
guitargeek added a commit that referenced this pull request Mar 7, 2026
For small unit tests like the RooFit pythonization tests, the overhead
of starting the Python interpreter is dominant. That means the tests
should not be too granular. I think testing the RooFit pythonizations
with separate Python executions at the level of the different RooFit
classes was indeed too granular. Therefore, this commit suggests to have
a single RooFit pythonization unit test.

Running the unit tests before (single thread):
```txt
Test project /home/rembserj/code/root/root_build/bindings/pyroot/pythonizations/test
      Start 64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabscollection
 1/14 Test #64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabscollection .......   Passed    1.09 sec
      Start 65: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabspdf-fitto
 2/14 Test #65: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabspdf-fitto ........   Passed    1.13 sec
      Start 66: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabsreal-ploton
 3/14 Test #66: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooabsreal-ploton ......   Passed    1.14 sec
      Start 67: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooarglist
 4/14 Test #67: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooarglist .............   Passed    0.93 sec
      Start 68: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roocmdarg
 5/14 Test #68: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roocmdarg ..............   Passed    0.95 sec
      Start 69: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-numpy
 6/14 Test #69: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-numpy ......   Passed    1.32 sec
      Start 70: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-ploton
 7/14 Test #70: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodatahist-ploton .....   Passed    1.09 sec
      Start 71: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset
 8/14 Test #71: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset .............   Passed    1.03 sec
      Start 72: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset-numpy
 9/14 Test #72: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roodataset-numpy .......   Passed    1.70 sec
      Start 73: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooglobalfunc
10/14 Test #73: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooglobalfunc ..........   Passed    1.21 sec
      Start 74: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roojsonfactorywstool
11/14 Test #74: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roojsonfactorywstool ...   Passed    1.40 sec
      Start 75: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roolinkedlist
12/14 Test #75: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roolinkedlist ..........   Passed    0.87 sec
      Start 76: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roosimultaneous
13/14 Test #76: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-roosimultaneous ........   Passed    1.09 sec
      Start 77: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooworkspace
14/14 Test #77: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit-rooworkspace ...........   Passed    1.21 sec

100% tests passed, 0 tests failed out of 14

Label Time Summary:
python                 =  16.16 sec*proc (14 tests)
python_runtime_deps    =   3.02 sec*proc (2 tests)

Total Test time (real) =  16.17 sec
```

With this commit:
```txt
Test project /home/rembserj/code/root/root_build/bindings/pyroot/pythonizations/test
    Start 64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit
1/1 Test #64: pyunittests-bindings-pyroot-pythonizations-pyroot-roofit ...   Passed    3.27 sec

100% tests passed, 0 tests failed out of 1

Label Time Summary:
python    =   3.27 sec*proc (1 test)

Total Test time (real) =   3.27 sec
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants