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

BUG : nbagg py3k compatibility #3464

Merged
merged 3 commits into from Sep 18, 2014
Merged

Conversation

tacaswell
Copy link
Member

Due to scoping fixes in py3k, list comprehensions over class level
attributes during class definition does not work (see
http://stackoverflow.com/questions/13905741/accessing-class-variables-from-a-list-comprehension-in-the-class-definition).

Superficially Fixes #3436. There seem to be other issues

Due to scoping fixes in py3k, list comprehensions over class level
attributes during class definition does not work (see
http://stackoverflow.com/questions/13905741/accessing-class-variables-from-a-list-comprehension-in-the-class-definition).

Superficially Fixes matplotlib#3436. There seem to be other issues
@tacaswell tacaswell added this to the v1.4.1 milestone Sep 3, 2014
@tacaswell
Copy link
Member Author

the backend will now start without error, but I get only a white space under the top bar with the red close button.

@tacaswell
Copy link
Member Author

cc @pelson

@jenshnielsen
Copy link
Member

The fix looks good to me. I would probably just have rewritten it as a regular loop but this is perhaps better. No clue about the blank screen

@tacaswell
Copy link
Member Author

Having the lists as class members seemed a tad too cute to me, as near as I could tell the list attributes never got re-used, and this seemed like the minimal change.

'zoom_to_rect': 'icon-check-empty',
'move': 'icon-move',
None: None
}

# Use the standard toolbar items + download button
toolitems = [(text, tooltip_text,
_font_awesome_classes[image_file], name_of_method)
Copy link
Member

Choose a reason for hiding this comment

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

How can this work? _font_awesome_classes now appears only here in this file, so as far as I can see, it is undefined. Overall, this block of code looks like a list incomprehension, not a list comprehension.

Copy link
Member Author

Choose a reason for hiding this comment

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

indeed, fixed it earlier today, but didn't get the commit pushed up till now.

@efiring
Copy link
Member

efiring commented Sep 4, 2014

OK, as a minimal change this should be fine. I still agree with Jens that this would be more readable, hence better, as a loop; but that would be just a style tweak.

When formatting the png data to send over the wire need to decode the
byte string to ascii.  If this is not done the literal string sent
to the browser is:
   "data:image/png;base64,b'iVBOR...'"
instead of
   "data:image/png;base64,iVBOR..."

The extra b' makes the string no longer a valid png which is why
we were getting white boxes
@tacaswell tacaswell changed the title BUG : fix list comprehensions over class members BUG : nbagg py3k compatibility Sep 6, 2014
pelson added a commit that referenced this pull request Sep 18, 2014
BUG : nbagg py3k compatibility
@pelson pelson merged commit cfc92e2 into matplotlib:v1.4.x Sep 18, 2014
@pelson
Copy link
Member

pelson commented Sep 18, 2014

Thanks @tacaswell!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants