Skip to content
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

make sure we only perform absolute imports on loading a backend #2335

Merged
merged 2 commits into from Sep 3, 2013

Conversation

andreabedini
Copy link

Fixes #2309

@andreabedini
Copy link
Author

Travis build failure is unrelated.

@@ -22,7 +22,7 @@ def pylab_setup():
backend_name = backend_name.lower() # until we banish mixed case
backend_name = 'matplotlib.backends.%s'%backend_name.lower()
backend_mod = __import__(backend_name,
globals(),locals(),[backend_name])
globals(),locals(),[backend_name],0)
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment explaining the reason for explicitly setting the import to be absolute. Also, I want to make sure that ipython and spyder does not break with this. Then, I want to get this patch merged asap to let it bake and hopefully we can catch anybody else that was depending on relative imports (can't imagine how, but just to be safe).

Copy link
Author

Choose a reason for hiding this comment

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

well, from line 23 it's clear that backend_name is an absolute module name. the difference relies on how user provided strings are understood. I'll see if I can add a note where this is documented.

@andreabedini
Copy link
Author

The documentation says ... if my_backend.pyis a matplotlib backend in yourPYTHONPATH ... to me this implies the import is meant to be absolute already.

@mdboom
Copy link
Member

mdboom commented Aug 27, 2013

Yeah -- I think absolute has certainly always been the intent, even if not the effect of the code.

@WeatherGod
Copy link
Member

Certainly... I just want a comment (or something) right there above it to explain the "0" parameter being passed to the __import__ function. It isn't like we use __import__ everyday. Lets imagine an incorrect value was placed there today, with no comment. A year from now, someone comes along and sees what the documentation says, and sees what the import does, and will probably be afraid to fix it because "it was set that way for some reason, right?"

Maybe it is just me being nuts...

@andreabedini
Copy link
Author

No, you're being perfectly reasonable. I'll write something in. 

Andrea


Sent from Mailbox for iPhone

On Tue, Aug 27, 2013 at 11:52 PM, Benjamin Root notifications@github.com
wrote:

Certainly... I just want a comment (or something) right there above it to explain the "0" parameter being passed to the __import__ function. It isn't like we use __import__ everyday. Lets imagine an incorrect value was placed there today, with no comment. A year from now, someone comes along and sees what the documentation says, and sees what the import does, and will probably be afraid to fix it because "it was set that way for some reason, right?"

Maybe it is just me being nuts...

Reply to this email directly or view it on GitHub:
#2335 (comment)

mdboom added a commit that referenced this pull request Sep 3, 2013
make sure we only perform absolute imports on loading a backend
@mdboom mdboom merged commit d0a0100 into matplotlib:master Sep 3, 2013
mdboom added a commit that referenced this pull request Sep 3, 2013
make sure we only perform absolute imports on loading a backend
@andreabedini andreabedini deleted the patch-5 branch September 3, 2013 22:02
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.

use('module://') directive doesn't work as expected
3 participants