Skip to content

ENH: allow building docs with Python 3 #450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 26, 2016
Merged

Conversation

grlee77
Copy link
Contributor

@grlee77 grlee77 commented Apr 30, 2016

  • commits 1 and 3 here make it so the docs can be builty with Python 3
  • the second commit is just misc PEP8 style changes

The third commit fixes ImportErrors such as the following when building with Python 3:

Generating docs for nibabel.benchmarks:
  -> nibabel.benchmarks
Traceback (most recent call last):
  File "tools/build_modref_templates.py", line 73, in <module>
    docwriter.write_api_docs(outdir)
  File "/media/Data1/src_repositories/my_git/nibabel_grl/doc/tools/apigen.py", line 473, in write_api_docs
    self.write_modules_api(modules, outdir)
  File "/media/Data1/src_repositories/my_git/nibabel_grl/doc/tools/apigen.py", line 437, in write_modules_api
    head, body = self.generate_api_doc(m)
  File "/media/Data1/src_repositories/my_git/nibabel_grl/doc/tools/apigen.py", line 276, in generate_api_doc
    functions, classes = self._parse_module_with_import(uri)
  File "/media/Data1/src_repositories/my_git/nibabel_grl/doc/tools/apigen.py", line 211, in _parse_module_with_import
    mod = __import__(uri, fromlist=[uri])
ImportError: No module named 'nibabel.benchmarks.nibabel'

@grlee77 grlee77 changed the title allow building docs with Python 3 ENH: allow building docs with Python 3 Apr 30, 2016
@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.85% when pulling 9c6543a on grlee77:docfix into 9909550 on nipy:master.

@grlee77
Copy link
Contributor Author

grlee77 commented Apr 30, 2016

The AppVeyor failure is unrelated (an md5 sum mismatch for the h5py package)

@arokem
Copy link
Member

arokem commented Apr 30, 2016

Known issue with appveyor: ContinuumIO/anaconda-issues#71

mod = __import__(uri, fromlist=[uri])
except ImportError:
# fix ImportError for submodules in Python 3
mod = __import__(uri, fromlist=[])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this will only import the base package module, so nibabel when uri='nibabel.parrec'. At least for me, you need fromlist=[''] to get the submodule, which I think is what we want. That seems to work for Python 2 and Python 3.

Copy link
Contributor Author

@grlee77 grlee77 May 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fromlist=[''] works fine on Python 3 for me, but with Python 2.7 I get the following error:

Generating docs for nibabel.freesurfer:
  -> nibabel.freesurfer
Traceback (most recent call last):
  File "tools/build_modref_templates.py", line 73, in <module>
    docwriter.write_api_docs(outdir)
  File "/media/Data1/src_repositories/my_git/nibabel_grl/doc/tools/apigen.py", line 470, in write_api_docs
    self.write_modules_api(modules, outdir)
  File "/media/Data1/src_repositories/my_git/nibabel_grl/doc/tools/apigen.py", line 434, in write_modules_api
    head, body = self.generate_api_doc(m)
  File "/media/Data1/src_repositories/my_git/nibabel_grl/doc/tools/apigen.py", line 273, in generate_api_doc
    functions, classes = self._parse_module_with_import(uri)
  File "/media/Data1/src_repositories/my_git/nibabel_grl/doc/tools/apigen.py", line 210, in _parse_module_with_import
    mod = __import__(uri, fromlist=[''])
  File "/home/lee8rx/anaconda/lib/python2.7/site-packages/nibabel/freesurfer/__init__.py", line 4, in <module>
    from .io import read_geometry, read_morph_data, write_morph_data, \
  File "/home/lee8rx/anaconda/lib/python2.7/site-packages/nibabel/freesurfer/io.py", line 8, in <module>
    from .. externals.six.moves import xrange
ImportError: No module named externals.six.moves

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a typo in the import line - extra space between the .. and externals.six.moves.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

strange. I still get the same error without the space. However, If I replace the relative imports like .. with absolute imports from nibabel. then it works fine for me. That doesn't really make sense, though, because the same sorts of relative imports are working fine from other files such as the ones in /benchmarks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm - that's odd. Sckit-image has a version of apigen.py - this is what they are doing : https://github.com/scikit-image/scikit-image/blob/master/doc/tools/apigen.py#L203

Would you mind adding a Python 3 version of the doc build and test to .travis.yml?

@codecov-io
Copy link

codecov-io commented May 24, 2016

Current coverage is 93.77%

Merging #450 into master will decrease coverage by <.01%

  1. 3 files (not in diff) in nibabel were modified. more
    • Hits -2
@@             master       #450   diff @@
==========================================
  Files           147        147          
  Lines         19181      19179     -2   
  Methods           0          0          
  Messages          0          0          
  Branches       2029       2029          
==========================================
- Hits          17986      17984     -2   
  Misses          796        796          
  Partials        399        399          

Powered by Codecov. Last updated by 7b9b83c...9c6543a

@grlee77
Copy link
Contributor Author

grlee77 commented May 24, 2016

The skimage solution is working on this laptop for python 3. I can rebase/merge a couple of the commits if this passes CI.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0004%) to 95.85% when pulling 59214b3 on grlee77:docfix into 9909550 on nipy:master.

grlee77 and others added 2 commits May 25, 2016 13:08
The fix was copied from scikit-image's version of apigen.py
@grlee77
Copy link
Contributor Author

grlee77 commented May 25, 2016

I squashed one of the commits, using the scikit-image solution for apigen.py. Thanks for finding that.

The last commit here fixes the following warning during the documentation build:

************
Advanced Testing
************
/media/Data1/src_repositories/my_git/nibabel_grl/doc/source/dicom/dicom_niftiheader.rst:73: SEVERE: Problems with "include" directive path:
InputError: [Errno 2] No such file or directory: 'source/dicom/links_names.txt'.
<autosummary>:1: ERROR: Unknown target name: "file".

However, I don't think any of the links from that file are actually being used in dicom-niftiheader.rst, so an alternative solution would be to delete the line instead.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.85% when pulling 0a6618a on grlee77:docfix into 7b9b83c on nipy:master.

@matthew-brett
Copy link
Member

Great - thanks. There's still a PEP style failure - but fine to merge after that's fixed...

@coveralls
Copy link

Coverage Status

Coverage remained the same at 95.85% when pulling d4b31e6 on grlee77:docfix into 7b9b83c on nipy:master.

@grlee77
Copy link
Contributor Author

grlee77 commented May 25, 2016

fixed the PEP8. Travis is happy now

@matthew-brett
Copy link
Member

Thanks for doing that - merging.

@matthew-brett matthew-brett merged commit 7e41691 into nipy:master May 26, 2016
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.

5 participants