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

Regression in transforms: raises exception when applied to single point#3866

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
pelson merged 3 commits intomatplotlib:masterfrommdboom:transform-single-point
Dec 3, 2014

Conversation

@mdboom
Copy link
Member

The following code snippet (taken from#3809) which transforms a single point used to work in v1.4.2 but does not work with latest master (it raises "ValueError: Expected 2-dimensional array, got 1"). Bisection of the history showed that the regression was introduced in commitbe34210 when PR#3646 was merged.

frommatplotlib.backends.backend_aggimportFigureCanvasAggasFigureCanvasfrommatplotlib.figureimportFigureimportnumpyasnpfig=Figure(figsize=(8,6))canvas=FigureCanvas(fig)ax=fig.add_subplot(1,1,1)ax.transData.transform((1,1))# <--- exception thrown here

Full traceback:

Traceback (most recent call last):  File "regression.py", line 9, in <module>    ax.transData.transform((1,1)) # <--- exception thrown here  File "/home/albert/work/code/matplotlib/lib/matplotlib/transforms.py", line 1289, in transform    return self.transform_affine(self.transform_non_affine(values))  File "/home/albert/work/code/matplotlib/lib/matplotlib/transforms.py", line 2259, in transform_affine    return self.get_affine().transform(points)  File "/home/albert/work/code/matplotlib/lib/matplotlib/transforms.py", line 1583, in transform    return self.transform_affine(values)  File "/home/albert/work/code/matplotlib/lib/matplotlib/transforms.py", line 1667, in transform_affine    return affine_transform(points, mtx)ValueError: Expected 2-dimensional array, got 1

@tacaswelltacaswell added this to thev1.5.x milestoneNov 30, 2014
@mdboom
Copy link
Member

Sorry -- I didn't realise this "feature" (which is a little dubious) was exposed all the way down to the public API. The attached patch should fix this.

Copy link
Member

Choose a reason for hiding this comment

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

Again, things I may not really want to know, but will ask anyway, what isO&O& vsOO&?

Copy link
Member

Choose a reason for hiding this comment

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

https://docs.python.org/3/c-api/arg.html#other-objects
TheO means dump the object in a C pointer (vertices_obj); theO& means use a converter function (convert_trans_affine) to convert the object before dumping the result in a C pointer (trans). This is the as-modified version; in the previous version, each of the two arguments was processed by a converter.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Exactly. In order to support the fact that the vertices array can be either 1-d or 2-d, I needed to do the conversion manually below rather than using a converter function.

@maxalbert
Copy link
ContributorAuthor

@mdboom Thanks for fixing this so quickly! Just out of interest, why did you say that this feature was a little "dubious"? From a user's perspective it seems nice that you can transform both a single point and a list of points (and this is what the documentation makes use of, too). Are you suggesting that it would be better to only allow transforming lists of points? From a user's point of view, having to write something likeax.transData.transform([(1,1)]) feels much more clumsy, though. Moreover, it may not be immediately obvious to new users what's wrong when they see the error message because intuitively they probably expect to be able to transform a single point.

Copy link
Member

Choose a reason for hiding this comment

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

Try catch, or just test the dimensionality of the object? Is there a preferred paradigm?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, I had the same thought when I read it. Testing for dimensionality seems slightly clearer to me personally because it doesn't treat the 1d case as "effectively wrong, but ok let's support it anyway". I don't have any strong opinions on this, though.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I was trying to avoid the 4-5 lines of complex code, involving reference counting, just to convert to an array to check its dimensionality, when that's neatly encapsulated in thenumpy::array_view constructor in an exception-safe way. If others feel strongly, I'm willing to change it, but I think this is less error-prone.

Copy link
Contributor

Choose a reason for hiding this comment

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

Again, no strong opinions from my side. It was just my first thought when I read the code, but with your explanation I'm happy to leave it this way. One question though, since I'm not really familiar with the C++ side of things: is it safe to catch a generic exception, or is there a way to specifically catch the ValueError that is triggered if the array is not 2d? I can't see much of a problem here, I'm just asking because I've been bitten by generic "catch" statements quite a few times in the past (but this was always on a much higher Python level with more potential for other exceptions to be triggered by the same piece of code, which doesn't seem to be the case here).

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I think in this specific instance, the generic catch is fine -- and we don't actually have much choice: All Python exceptions "thrown" aspy::exception on the C++ side anyway.

@pelson
Copy link
Member

👍 but for the test from me.

@mdboom
Copy link
Member

It's dubious because the return type depends on the input type, and the input type is not very well defined. I think the method for transforming a single point should probably be a separate method, but that would break backward compatibility for no major gain...

pelson added a commit that referenced this pull requestDec 3, 2014
Regression in transforms: raises exception when applied to single point
@pelsonpelson merged commit77ccc82 intomatplotlib:masterDec 3, 2014
@mdboommdboom deleted the transform-single-point branchMarch 3, 2015 18:44
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

@mdboommdboom

Projects

None yet

Milestone

v2.1

Development

Successfully merging this pull request may close these issues.

5 participants

@mdboom@maxalbert@pelson@efiring@tacaswell

[8]ページ先頭

©2009-2025 Movatter.jp