docs do not build on py3.7 due to small change in python handling of -m #11838

Activity

1. tacaswell

   added this to the v2.2-doc milestone on Aug 10, 2018
2. ImportanceOfBeingErnest commented on Aug 10, 2018

   

   ImportanceOfBeingErnest

   on Aug 10, 2018

   Member

   More actions

   I do not quite understand how that -m issue affects this. I always thought examples are run through sphinx gallery, which would os.chdir to the directory of the example file. In that case importing from the cwd should work fine, no?
3. tacaswell commented on Aug 10, 2018

   

   tacaswell

   on Aug 10, 2018

   MemberAuthor

   More actions

   Sorry my notes from last night were not super clear.

   • The MakeFile we use invokes the build via

     matplotlib/doc/Makefile

     Line 6 in cff3da2

     SPHINXBUILD = python -msphinx

   • previously when using -m the string '' was first in the path which means the cwd of the process is always in the path (even if you change directories)
   • we, via sphinx-gallery, rely on this when we exec the example code to have the local imports work
   • the fix in cpython to the security concern is that the 'cwd' is resolved as the directory you are launching from and that absolute path is put into sys.path. This in turn breaks our imports

   ex

   (dd36) ✔ /tmp 
   13:05 $ python -m test_mod
   3.6.6 | packaged by conda-forge | (default, Jul 26 2018, 09:53:17) 
   [GCC 4.8.2 20140120 (Red Hat 4.8.2-15)]
   ['', '/home/tcaswell/mc3/envs/dd36/lib/python36.zip', '/home/tcaswell/mc3/envs/dd36/lib/python3.6']
   (dd36) ✔ /tmp 
   13:09 $ conda activate dd37
   (dd37) ✔ /tmp 
   13:09 $ python -m test_mod
   3.7.0 | packaged by conda-forge | (default, Jul 23 2018, 21:39:36) 
   [GCC 4.8.2 20140120 (Red Hat 4.8.2-15)]
   ['/tmp', '/home/tcaswell/mc3/envs/dd37/lib/python37.zip', '/home/tcaswell/mc3/envs/dd37/lib/python3.7']
   (dd37) ✔ /tmp 
   13:09 $ cat test_mod.py 
   import sys
   print(sys.version)
   print(sys.path[:3])
   (dd37) ✔ /tmp 
   
   

   I think the fix is to in our sphinx conf add if '' not in sys.path: sys.path.append('') (or something to that effect) or to move this logic to sphinx gallery (and have it explicitly add / remove the cwd directory as it moves around the examples).
4. ImportanceOfBeingErnest commented on Aug 10, 2018

   

   ImportanceOfBeingErnest

   on Aug 10, 2018

   Member

   More actions

   There is already a similar hack present in conf.py

   matplotlib/doc/conf.py

   Line 22 in f4ae5d1

   sys.path.append(os.path.abspath('.'))

   so that might be the best option for now.
5. anntzer commented on Aug 10, 2018

   

   anntzer

   on Aug 10, 2018

   Contributor

   More actions

   From a quick look at it, I'd suggest consolidating all unit examples that import basic_units into a single one together with basic_units itself; having each "this specific plotting method supports units" example in a separate file doesn't seem to be that helpful(?)
6. tacaswell

   added a commit that references this issue on Sep 18, 2018

   BLD: pragmatic fix for building basic_unit example on py37

   ...

   This issue will close once commit 6049f13 is merged into the 'main' branch.

   6049f13
7. jklymak

   closed this as completedin #12148on Sep 18, 2018