Skip to content

MRG: Py3k support #152

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 4 commits into from
May 17, 2016
Merged

MRG: Py3k support #152

merged 4 commits into from
May 17, 2016

Conversation

larsoner
Copy link
Contributor

Brings PySurfer py3k support. No CI support yet because Anaconda packages don't exist yet. But tests pass on my system with Mayavi dev and VTK 7.0.

Closes #143.

@agramfort
Copy link
Contributor

Can you build the mne doc with it?

@larsoner
Copy link
Contributor Author

I have some annoying backend conflicts with matplotlib when I try to do it, but I don't think it's PySurfer's problem


needed_deps = ["IPython",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know why we had IPython listed as a dependency... @mwaskom looks like this was your change any idea?

Copy link
Member

Choose a reason for hiding this comment

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

The pysurfer command line tool drops you into IPython. Also in general you need IPython to do any interactive work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The pysurfer command line tool drops you into IPython.

Then it should be an optional dependency, not a hard requirement. People (like me) who do import surfer and do everything from there shouldn't need to have IPython.

Also in general you need IPython to do any interactive work.

I disagree -- I use python -i (effectively) from Spyder in general, and rarely (if ever) use IPython, and have no problems with interactivity.

Copy link
Member

Choose a reason for hiding this comment

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

Ok. I don't particularly care about declaring it as a dependency...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool. Any other comments? If not I think it should be good to merge.

Copy link
Member

Choose a reason for hiding this comment

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

LGTM!

@coveralls
Copy link

coveralls commented May 16, 2016

Coverage Status

Changes Unknown when pulling a079c22 on Eric89GXL:py3k into * on nipy:master*.

@agramfort
Copy link
Contributor

LGTM

@larsoner larsoner merged commit 40621da into nipy:master May 17, 2016
@larsoner larsoner deleted the py3k branch May 17, 2016 01:56
@larsoner
Copy link
Contributor Author

Thanks for the quick reviews. Now everyone gets to have fun building the VTK and Mayavi :)

@mwaskom
Copy link
Member

mwaskom commented May 23, 2016

Forgot to say cheers for doing the boring work here!

(But also, sotto voce boos for removing my main excuse for not switching to Python 3).

@agramfort
Copy link
Contributor

agramfort commented May 23, 2016 via email

@mwaskom
Copy link
Member

mwaskom commented May 23, 2016

That's my current supplementary excuse. I haven't actually tried to use it yet :)

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