-
Notifications
You must be signed in to change notification settings - Fork 437
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
Modifications for building docs with python3 #1091
Conversation
Current coverage is 80.84% (diff: 100%)@@ master #1091 diff @@
==========================================
Files 200 200
Lines 23023 23023
Methods 0 0
Messages 0 0
Branches 2309 2309
==========================================
Hits 18614 18614
Misses 3933 3933
Partials 476 476
|
Nice! But documentation still does not build on Python 3 with this fix. Still need to also adjust this file to be Python 3 compliant: https://github.com/nipy/dipy/blob/master/doc/sphinxext/github.py This line, for starters: https://github.com/nipy/dipy/blob/master/doc/sphinxext/github.py#L39 (that's where I get an error raised) |
I don't get it on make api or make html
Any suggestions how should I test it?
|
I am not sure. Could it be that your system is getting the |
Either way, it's still not Python 3 compliant. Try:
|
@arokem yes I will fix that file. I have to check why I am not getting the error. |
@arokem sorry my bad. I was using python2 sphinx as default. I found that numpydoc extension also crashes. I think removing it from the sphinxext folder will be better as we can use the lastest numpydoc without shipping an older version. What do you recommend? |
You can also get python 3 compliant versions of all these here: https://github.com/uwescience/shablona/tree/master/doc/sphinxext |
@arokem okey then I will include those. |
Looks good. Thanks @ghoshbishakh |
@Garyfallidis Not much is left. But I have not been able to test it completely with python3 as it takes a long time. You may merge it as I can make patches later also. |
Why does it take a long time? |
@Garyfallidis Because I have only 4GB RAM and the build process requires about 16GB. And using swap it takes about 20 hours to build the entire doc. |
I think there are only a few examples that need more memory than usual. DKI and Life. You may want to disable them by commenting them at doc/examples/valid_examples.txt |
@Garyfallidis ok. I will build it and post a link here. |
I am getting this error:
|
Hi @MarcCote have you seen this? Any ideas? |
I ran the code for segment_clustering_features.py separately and I get this:
|
@ghoshbishakh: @Garyfallidis just told me about your issue with |
What did just happen? I merged @MarcCote 's PR and pulled it locally and then made a push and this happened. 😟 |
@MarcCote seems I have made a successful reset! Can you somehow make the PR again? |
You might want to both be rebased on top of current master before trying On Sat, Jul 23, 2016 at 7:18 AM, Coveralls notifications@github.com wrote:
|
That PR solved that issue @MarcCote 😄
|
Here is the output if I run streamline_formats.py separately:
|
looks like changing
to
fixes it. Got some clue from here: |
@arokem can you suggest me if I should make that change in the string in this PR? |
Where is this change implemented? In |
@arokem yes the change is in And yes it works in both python2 and 3 and the output is:
|
Then yes - I say we should include it in this PR. @Garyfallidis : any thoughts here? |
Sorry - let me walk back that last comment. Thinking about this for one more second: I think this should be a separate PR. In particular, I would like to see a test that fails on current master, and is fixed when this change is introduced. |
That should make for a really small PR, which we can merge quickly. Then, you can rebase this current PR on top of that change and move ahead with it. How does that sound? |
@arokem Ok I am making a separate PR quickly |
OK - now that #1100 is merged, you can rebase this one and keep going here. |
And minor PEP8 changes fixes dipy#912
Taken from @arokem 's commit: uwescience/shablona@ee63354
Now there is a problem with restore_dti.py
|
@arokem now this works with all examples enabled except :
I have not checked for these two |
Why are you not running these? The DKI example does rely on downloading a large amount of data (we need data with multiple b-values...). I just ran it on Python 3 on my laptop and it runs fine. The LiFE example ( |
I ran LiFE example and it does take a lot of ram and my pc is stuck now. I Just out of curiosity.. I noticed some examples use all the 4 cores to On Jul 31, 2016 11:41 PM, "Ariel Rokem" notifications@github.com wrote:
|
Did you run the If you have the file 'lr-superiorfrontal.trk' in place already (that is, As for full utilization of cpu: some of our code already uses On Sun, Jul 31, 2016 at 11:33 AM, Bishakh Ghosh notifications@github.com
|
@arokem I just ran the example separately this time and it worked.
|
Excellent! I think this is probably ready to go then, right? Anyone else want to take a look? |
@arokem yes it is working perfectly. Let others take a look for a couple of days. |
Change apigen.py for python3 support
And minor PEP8 changes
fixes #912