-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
sphinxext py3 compatibility #5530
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
sphinxext py3 compatibility #5530
Conversation
I can't test this at the moment, but if it solves the python 3 issues with the docs for the moment, good for me! |
Are these tests skipped by travis? |
we don't test doc builds with Travis. It's difficult to do and it fails for silly reasons (like latex not being installed) |
But we could test this specific file (it just requires sphinx) ? |
Is this file skipped? I don't see it anywhere in the Travis log (which lists the skipped modules). If it is skipped I could try adding it, but I don't see how - the script.sh file that gets called by travis looks pretty inclusive:
With the Anyway, this commit solves some of the issues (enough to pass the test_docscrape.py on the latest versions of Python 2 and 3 on my machine), but not all, so @phaebz (a colleague of mine, I added him as a contributor to the branch) and I will probably contribute more fixes to this branch. |
I guess it must be skipped as other branches here pass on python 3! It could be the pandas in the middle (does this only run tests within pandas directory? and not any in doc/sphinxext) ...Obviously passing that test won't confirm that the docs build on python 3, but this could be a start/prevent regressions :) That you guys are working on this is great! |
it's only running the pandas directory. that test is under docs. But it needs to only run if sphinx is installed, no? |
So the question is should we run this (currently) small test to imply the docs are buildable. Perhaps this is out of scope of what travis should be doing usually... |
I had a look at the Now I want to just use the import logic, namely the try/except part of |
Not clear what the issue is, if this is in pandas repo you can assume pandas is installed. Therefore, you can use compat. |
Nevermind, I think I figured it out - see upcoming commit(s?). My question stemmed from the fact that I am new to compat issues between python 2 and python 3 in terms of code base organisation as big as pandas. |
... or not figured it out. Concrete example: I want to make
which looks like a "private" by the underscore convention (?). |
It's not the module, you actually get the object to instantiate, eg: from pandas.compat import cStringIO buffer = cStringIO() |
|
I tried to use as much
Under Python 3 sphinx runs through and says docs built successfully, but I get a ton of autodoc/docstring errors related to StringIO, xrange and pandas submodule imports
|
Are you working on Windows? In that case, the first error (on python 2 causing the debugger to start) could possibly attributed to io.rst (there is a unicode example that does not build on windows/python 2, see #5142). |
I am working on OS X 10.8.5. I had a look around in the debugger and it turns out that indeed the >>> import locale
>>> print(locale.getlocale())
('en_US', 'UTF-8')
>>> print(locale.getdefaultlocale())
('en_US', 'UTF-8') |
@phaebz Something else, I tried to build the docs with the IPython sphinxextensions, and it worked with some warnings (on Python 2, but they should be python 3 compatible). This could also be a way to, at the moment, build the docs on python 3. Replace
with
You need IPython 1.0 (or larger) for this. |
Thanks for letting me know, this serves ok for now. I will use this locally from now on when working on docs. As this PR was intended to be a temporary fix before #5221 made it through / while it is discussed I will check back with @Kermit666 how/if we will proceed. |
@Kermit666 how's this coming along? |
@jreback currently lost in some "maximum recursion depth" errors I didn't expect to encounter in a Python 2 to 3 conversion:
I'll try to work on it some more to figure it out. |
That is the same error as @cancan101 got, and I also saw it once (I was thinking it maybe something to do with sphinx 1.2 vs 1.1.3, but I still have to test that) |
@Kermit666 See #5899, do you use Sphinx 1.2? And can you try with 1.1.3? |
Thanks @jorisvandenbossche , that seems to be the root. I tried with Sphinx 1.1.3, but that one has some other errors (possibly py3-related) and now I'm working on the development version of Sphinx (1.3a). I am also getting weird errors with pandas itself.
I even tried rebasing to the latest master, but that didn't help. Is master unstable at the moment (in which case - should I be working off of the latest tag instead?) or am I maybe missing something locally? |
Normally master should be stable. Did you build pandas again after rebasing? (eg And with the development version of Sphinx (1.3a), is the 'maximum recursion error' solved? |
OK, you are right @jorisvandenbossche . I thought that Yes, the dev version of Sphinx resolves the 'maximum recursion error'. As far as the documentation itself is concerned - on the current branch I am able to generate it, but I get a ton of warnings and import errors along the way (import errors for missing scipy, xlrd,..., missing file 'foo.xlsx', toctree warnings). This whole build process seems very complex... I expected something more in the line of mark-up -> html :) |
@Kermit666 The reason building the docs seems complex (lot of errors and warnings etc) is because all code examples in the docs are run and the output saved during the docs build (using the ipython directive, so it's not simply rst -> html, the code examples in rst are first generated). This also means that, if you don't have all optional dependencies for pandas (hdf5-related, html-related, excel-related, etc) some of the code examples will generate errors. |
OK, thanks. I'll play with it tonight some more with all the optional dependencies then. |
@jorisvandenbossche I adapted the test for |
Tested it with Windows/python 2, and I also get a UnicodeError while parsing io.rst (as before @y-p's unicode fix 0543a21). This was with just using readlines instead of buflist (metakermit@5165a53). Now, I don't really get my head around what this buflist actually is/does. It is in every case not the same as readlines (just try simply To also getting it to work on python3 / windows, we would need a python 3 equivalent of @y-p unicode fix 0543a21. |
This PR has been troubling me and I've been giving some thought as to why. Here's my take:
The last point is crucial. Building the docs currently takes a long time. Cross-checking So if you can make the docs build on py3 without intrusive changes, that's great. as an aside, two suggestions on how to make a significant contribution
Possible ways to go about achieving the latter are:
|
re the It's an ugly hack, which is also why I'm not going to submit it to upstream ipython. |
@y-p I completely agree with the fact that contributing to the docs should be as easy as possible, but some thoughts:
So I think that the changes are not that intrusive (and are already there in the docs). And we can say that we don't expect from new contributions that they have checked on python3. If we see an obvious one (like a print statement), we can comment, and for the rest it is up to the contributors using python 3 to build the docs once in a while and push needed changes? What I propose:
I think with those two fixes, the docs build with python 3 on linux (correct @phaebz @Kermit666 ?). Thoughts? And further leave the buflist / windows decode error as is for the moment (unless someone wants to dive into that). |
I concur with every single item. No problem bypassing the buflist fix on py3 if you wish. Having py3 for utf8 systems beats having All the rest is completely reasonable as well. bombs away. |
BTW, (I am not a python 3 expert) but is this Eg |
@Kermit666 @phaebz If someone of you can adapt the commits here to the two points in my previous comment, we can move forward and merge this :-) (There is still the issue of rebuilding, but leave that for another PR, it's getting long enough here) |
It's not required on py3 nor on py2.
It's a bit ugly, yeah. But saves the 2to3 pass. |
I understand that it is used like that internally, but is needed in the docs? As I can, as a user, just type |
|
@jorisvandenbossche RE: the If this is ok with you guys I will talk to @Kermit666 and we do the squashing. |
No, I think it's only removed in python 3 (I have python 2.7.6 and there StringIO has buflist and the fix works). The |
you are right, I did not instantiate correctly when I tried it. Amended. |
…tributeError on Pandas.str
OK, I squashed the branch into 2 commits regarding the relevant changes and rebased to master so that you can easily merge it. @y-p I agree with the stuff you said. The last thing we want is to fend off potential contributors for being too zealous. Also, we'll try to consider the things you suggested for future improvements when picking the next thing to work on ;) |
Thank you, I appreciate your cooperation. |
@Kermit666 @phaebz Super. Just a last small comment: at the And are you going to try to push the numpydoc fix upstream? |
I just dropped b3297dd since @y-p moved the relevant section in 438d614 anyways |
Yes indeed, merging this then. Can you also try out @y-p new try on the unicode issue on python 3 (I tested it on windows, but not yet python 3). @Kermit666 @phaebz Thanks again for your effort on this! |
sphinxext py3 compatibility
@phaebz I was just thinking, the |
@jorisvandenbossche yes, the except AttributeError shouldn't be necessary any more, but I think it does no harm in case an error such as this one pops again in the future. I sent a PR upstream just now (numpy/numpydoc#11). |
Temporary sphinxext py3 compatibility fix as discussed in #5479 before #5221 is resolved. So far test_docscrape.py passes on both Python 2.7 and 3.3 (which should be the typical developer interpreters).