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

Issue 2899#2923

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 7 commits intomatplotlib:masterfromsfroid:issue_2899
Mar 27, 2014
Merged

Issue 2899#2923

efiring merged 7 commits intomatplotlib:masterfromsfroid:issue_2899
Mar 27, 2014

Conversation

sfroid
Copy link
Contributor

#2899

Adding wedgeprops and textprops arguments to pie, which are forwarded to the wedge and text objects respectively. This allows the user to control various properties of the pie directly (eg linewidth of the border of wedges).
Added image test to check linewidth of pies (for cases linewidth 0 and 2).

@sfroid
Copy link
ContributorAuthor

@tacaswell@mdboom
Need some pointers on how to resolve failing tests

  1. The autogenerated pyplot.py file has the prefix "u" appended to all string type function arguments. I can manually edit these to remove the prefix "u", but that would defeat the purpose of an autogenerated file. How do I fix this?
  2. I am getting an asserting error, which seems to be unrelated to the changes I made...
======================================================================FAIL: Github issue #1256 identified a bug in Line.draw method----------------------------------------------------------------------Traceback (most recent call last):  File "/home/travis/virtualenv/python2.6/lib/python2.6/site-packages/nose/case.py", line 197, in runTest    self.test(*self.arg)  File "/home/travis/virtualenv/python2.6/lib/python2.6/site-packages/matplotlib-1.4.x-py2.6-linux-x86_64.egg/matplotlib/testing/decorators.py", line 110, in wrapped_function    func(*args, **kwargs)  File "/home/travis/virtualenv/python2.6/lib/python2.6/site-packages/matplotlib-1.4.x-py2.6-linux-x86_64.egg/matplotlib/tests/test_lines.py", line 58, in test_invisible_Line_rendering    assert_true(slowdown_factor < slowdown_threshold)AssertionError

Any idea what could be going wrong?

It seems to me as if this test could fail depending on how long it takes for the rendering function. It probably is not something caused by my changes.

@tacaswell
Copy link
Member

I would ignore those test failures, that one pops up from time to time (free cpu cycles are not guaranteed cpu cycles).

The problem with the unicode strings come from thefrom __future__ import unicode_literals at the top of every file which is part of the python2/python3 compatibility layer. boilerplate.py does some introspection and then generates pyplot.py and (correctly) writes out all of the strings as unicode. However, this ends up being a problem because in python 3 the default in unicode strings and in 3.2 it will raise an error if you give it au'blah' string. I think last time this came up I appliedsed to the output....

@sfroid
Copy link
ContributorAuthor

@tacaswell
This is ready to be merged... its now on your plate, whenever you have the time.

I am now starting to look at the needs_patch issues.

@tacaswelltacaswell added this to thev1.4.0 milestoneMar 26, 2014
@tacaswell
Copy link
Member

This looks great! Thanks for sorting out the issue with boilerplate.

I am happy with these changes, but I would like@mdboom or@efiring to take a look at the changes to boiler plate.

I also don't recall of the top of my head if we want to keep all of the images for all of the tests (png might be enough).

a plotting method from pyplot, edit the appropriate list in `boilerplate.py`
and then run the script which will update the content in
`lib/matplotlib/pyplot.py`. Both the changes in `boilerplate.py` and
`lib/matplotlib/pyplot.py` should be checked into the repository.

Note: boilerplate.py looks for changes in the installed version of matplotlib
Copy link
Member

Choose a reason for hiding this comment

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

Five years ago, ine91c5a1, I commented out the line that was making boilerplate look first in the local tree, without removing the comment that went with that line. I don't know why I did that. If the present behavior is what we want, then the first two lines in the excerpt from boilerplate, below, should be deleted. I suspect the present behavior makes sense because otherwise python code would be found in the source tree but compiled extensions would be from the installed version, so they could be out of sync.

# import the local copy of matplotlib, not the installed one#sys.path.insert(0, './lib')frommatplotlib.axesimportAxes

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

@efiring
Updated boilerplate.py with a better comment.

@sfroidsfroid mentioned this pull requestMar 26, 2014
@efiring
Copy link
Member

Travis PEP8 failures are unrelated to this PR. Were modules such as animation and patches previously exempt from PEP8 testing, but recently included? Or are more PEP8 tests now being used as criteria for rejection? Or did a merge accidentally revert the modules to a much earlier state? I don't think it is the last of these, fortunately.

@tacaswell
Copy link
Member

Oddly, one of the builds (2.7) seems to have passed even though the others failed for pep8 reasons.....

@sfroid
Copy link
ContributorAuthor

@efiring@tacaswell
The travis failure was indeed unexpected as my changes passed the Travis tests yesterday (after the second last commit). My last looks completely harmless/unrelated to any kind of tests.

I have no idea of any modules being PEP8 exempt. I jumped on the matplotlib dev bandwagon last week and this is literally my first PR to this repo. Also, I don't see any merge related changes in the diff for this PR.

@tacaswell
Copy link
Member

@sfroid See#2930 . The issue is that pep8 bumped it's version today which seems to have introduced a bunch of new tests. I suspect the one that passed got a cached copy of the old pep8 install.

@sfroid
Copy link
ContributorAuthor

@tacaswell@efiring
What is the suggested solution here? Fix all the pep8 reported issues (334 in all, but each of them quite simple)?
Or do we have another way to handle this?

@tacaswell
Copy link
Member

The other PR squelches the tests that we did not run before.

Please don't start fixing pep8 issues in this PR, if we are going to do that we should do it in a dedicated PR. Style conflicts are the worst to merge.

efiring added a commit that referenced this pull requestMar 27, 2014
@efiringefiring merged commitbe33461 intomatplotlib:masterMar 27, 2014
@sfroid
Copy link
ContributorAuthor

Yayy!! My first contribution to matplotlib.
Thank you@tacaswell and@efiring for the help.

@sfroidsfroid deleted the issue_2899 branchMarch 27, 2014 21:25
@efiring
Copy link
Member

Thank you@sfroid for the contribution.

@tacaswell
Copy link
Member

Thank you@sfroid !

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.4.0
Development

Successfully merging this pull request may close these issues.

4 participants
@sfroid@tacaswell@efiring@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp