Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Pickling support added. Various whitespace fixes as a result of reading *lots* of code.#1175

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
efiring merged 3 commits intomatplotlib:masterfrompelson:pickle2
Sep 1, 2012

Conversation

pelson
Copy link
Member

Rebasing#1020 was causing real problems, therefore I decided to cut a fresh branch and merge my changes on (with a commit squash to boot).

@dmcdougall
Copy link
Member

I see changes to colorbar and legend! Does this mean you've added support for them?

@pwuertz
Copy link
Contributor

This newnew_figure_manager_given_figure method should be added to backend_pgf.py as well, right?

@travisbot
Copy link

This pull requestfails (merged 51f8ffc6 intocf7618c).

@pelson
Copy link
MemberAuthor

@dmcdougall: Yes. ;-)
@pwuertz: Yes. ;-)

Will do the pgf backend now.

@travisbot
Copy link

This pull requestfails (merged7885d94 intocf7618c).

if getattr(self.canvas, 'manager', None) is not None:
manager = self.canvas.manager
import matplotlib._pylab_helpers
if manager in matplotlib._pylab_helpers.Gcf.figs.viewvalues():
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

based on the travis results, I am assumingviewvalues is not python2.6 compatible?

Copy link
Contributor

Choose a reason for hiding this comment

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

@pelson
Copy link
MemberAuthor

I have been developing this with python2.7, but it appears that this is not compatible with python3.2 nor python2.6. I don't have local installations of either and would have to build both from source, if anyone else is willing to get involved I would much appreciate it, otherwise it looks like I have a painful journey of building ahead of me...

@pwuertz
Copy link
Contributor

I have a few python 3 fixes for you in this commit:
https://github.com/pwuertz/matplotlib/commit/0e7c6e3e65c8559254d650f9307d13ec544ba57c

Successfully pickled and unpickled a simple figure with python3 using that patch on top.

@pwuertz
Copy link
Contributor

The figure from your test_complete also pickles/unpickles fine with python3, but apparently you cannot unpickle data that has been pickled with python2...

@pelson
Copy link
MemberAuthor

@pwuertz: awesome! I've cherry picked that commit into this PR.

@pwuertz
Copy link
Contributor

Ah, hold on. In python3 you need a Byte buffer instead of a String buffer for pickle. This commit here makes the test module work.
https://github.com/pwuertz/matplotlib/commit/8753aaa894c561a8b79dacccc713e7302030fda1

I don't know if the code is now further away from being python2.6 compatible.

Also, our python editors seem to have a fight over a few whitespaces :)

@mdboom
Copy link
Member

As for testing on different versions of Python -- hopefully the Travis bot will start to be helpful with that. We have a number of failures on master at the moment, so it makes everything look like it's failing -- but manually expecting the results should hopefully be enough to track down whether a given pull request introduces anynew failures.

@travisbot
Copy link

This pull requestfails (mergedf4245bb intocf7618c).

@dmcdougall
Copy link
Member

I'm confused. How do I use travis bot? The web interface sure looks pretty, but I do not understand where to see what tests are failing where. Is there documentation?

@travisbot
Copy link

This pull requestfails (merged1e08190 intocf7618c).

@pelson
Copy link
MemberAuthor

@pwuertz: Thanks for the added commit. I will need to do a bit of work to make that code python2 compatible, but that's fine. Mightn't be until Monday now though.

@pelson
Copy link
MemberAuthor

Given this is quite a significant code change I would be eager to merge this and then fix the python3 support in a subsequent PR. Some of the benefits of doing this:

The biggest drawback of doing this:

  • Currently, the pickle test will fail on python3 (and possibly python2.6)

Is anyone willing to merge this?

@@ -608,7 +609,14 @@ def new_figure_manager(num, *args, **kwargs):
# main-level app (egg backend_gtk, backend_gtkagg) for pylab
FigureClass = kwargs.pop('FigureClass', Figure)
thisFig = FigureClass(*args, **kwargs)
canvas = FigureCanvasPgf(thisFig)
return new_figure_manager_given_figure(thisFig)
Copy link
Member

Choose a reason for hiding this comment

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

Don't you need a "num" argument here, to match the signature below?

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Yes. Thanks Eric.

efiring added a commit that referenced this pull requestSep 1, 2012
Pickling support added. Various whitespace fixes as a result of reading *lots* of code.
@efiringefiring merged commit4c1e36d intomatplotlib:masterSep 1, 2012
@efiring
Copy link
Member

With some trepidation, but in the interest of keeping things moving, I went ahead with the merge. I suspect that will wreak havoc with many other pull requests, but being small, they will not be difficult to fix.

@dmcdougall
Copy link
Member

Woohoo! This has me a littletoo excited. Good work!

@pelson
Copy link
MemberAuthor

@efiring: Thanks for doing that: I think it is a good decision. It will certainly save me time in having to rebase (and other re-read), which I intend to invest to other mpl related things. As you can see, I have created a new PR with the feedback from this PR applied.

@dmcdougall : Thanks for the back pat. Its good to know that these features are appreciated. Do you have a particular use in mind for pickle support or is it just a general purpose "nice to have"?

@dmcdougall
Copy link
Member

@pelson Well, it is 'nice to have'. Though, in particular, it would save me a lot of time when I write a script that generates 8 figures from several 2GB data files and then a co-author tells me that there are too many yticks.

@pelson
Copy link
MemberAuthor

Though, in particular, it would save me a lot of time when I write a script that generates 8 figures from several 2GB data files and then a co-author tells me that there are too many yticks.

Yes. I like it. ;-) Your right: it is the general purpose - little things which will save a little bit of time time - benefits that I see this delivering most of all.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.2.x
Development

Successfully merging this pull request may close these issues.

7 participants
@pelson@dmcdougall@pwuertz@travisbot@mdboom@efiring@astrofrog

[8]ページ先頭

©2009-2025 Movatter.jp