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

Improve docstring of Axes.imshow#10982

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
jklymak merged 1 commit intomatplotlib:masterfromtimhoffm:doc-axes-imshow
May 7, 2018

Conversation

timhoffm
Copy link
Member

@timhoffmtimhoffm commentedApr 8, 2018
edited
Loading

PR Summary

As part of#10148: Docstring update forAxes.imshow.

  • The parameterresample was not documented. From the code, I couldn't really find out what it does.
  • The parametersshape andimlimare unused and should probably be removed (would do that in a separate PR once this is merged).
  • The parameterfilternorm should probably be a bool all throughout the class hirarchy (separate PR as well).
  • Should we change the parameterX forZ as is the case in matshow?Z makes a bit more sense, however, technically this would be a breaking change if a user has used it as kwargimshow(X=data)

@anntzer
Copy link
Contributor

See#10442 for making filternorm a bool.

@ImportanceOfBeingErnest
Copy link
Member

In line 5177

By default, the center of the lower left
pixel is at (0, 0) and pixels have a size of (1, 1), i.e.extent
would be (0, columns-1, 0, rows-1).

is incorrect in two ways.

  • extent is the edges of the pixels. Hence it starts at-0.5 and goes tocolumns-0.5
  • There is something tricky about the interplay between extent and origin.origin defaults toupper (I think) and in that case the extent in y direction needs to to be reversed. See following example:
a = np.triu(np.random.randn(5, 5))a[a==0] = np.nanfig, (ax,ax2,ax3,ax4) = plt.subplots(ncols=4)extent_upper=[-0.5,a.shape[1]-0.5,a.shape[0]-0.5,-0.5]   # <-- here y-extent needs to be invertedax.imshow(a, cmap="Blues", origin="upper")ax2.imshow(a, cmap="Reds", origin="upper", extent=extent_upper)extent_lower=[-0.5,a.shape[1]-0.5,-0.5,a.shape[0]-0.5]ax3.imshow(a, cmap="Blues", origin="lower")ax4.imshow(a, cmap="Reds", origin="lower", extent=extent_lower)plt.show()

image

corner of the axes. If None, default to rc `image.origin`.
corner of the axes. The convention 'upper' is typically used for
matrices; 'lower' is typically used for image data. Defaults to
:rc:`image.origin`.

Choose a reason for hiding this comment

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

I would say imagesare matrices. At least the convention is that images have the first pixels in the upper left corner.

image

Maybe better

The convention 'upper' is typically used for
matrices and images; 'lower' is typically used for displaying data arrays.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

What are data arrays? numpy also uses the 'upper' convention:

>>> print(np.array([[1, 2], [3, 4]]))[[1, 2], [3, 4]]

For now, I don't have a good example for 'lower', so I just say

The convention 'upper' is typically used for matrices and images.

and don't give an example for lower.

@timhoffm
Copy link
MemberAuthor

Description forextent corrected.

@ImportanceOfBeingErnest
Copy link
Member

By default, the center of the lower left
pixel is at (0, 0) and pixels have a size of (1, 1), i.e.extent
would be (-0.5, columns-0.5, -0.5, rows-0.5).

This is still not correct. The default origin is "upper" via the rcParams. Hence the lower left pixel is at (0,rows) (see left-most plot above). To achieve this plot by setting an extent you need[-0.5,a.shape[1]-0.5,a.shape[0]-0.5,-0.5] (as seen in the second plot).

In case the origin is "lower", the sentence is correct. (c.f. the right two plots above).

To be on the safe side here, maybe the only option is to clearly state both possible cases.

By default, pixels have a size of (1, 1).
Note that the extent is dependent on theorigin setting.
Iforigin is set toupper, the center of the lower left
pixel is at (0, rows) , this corresponds to anextent
of (-0.5, columns-0.5, rows-0.5, -0.5).
Iforigin is set to"lower", the center of the lower left
pixel is at (0, 0). This is equivalent to anextent
of (-0.5, columns-0.5, -0.5, rows-0.5).

I guess ideally there should be a link to an example showing all possible cases.

@phobson
Copy link
Member

I'm inclined to avoid the word "matrix" altogether since thenumpy.matrix class is such a confusing mess and I'd like to avoid steering people in that direction. I propose to replace "matrix" with "2D-array". The term "raster" is all likely a appropriate, but I associate it (rightly or not) with GIS data.

@timhoffm
Copy link
MemberAuthor

@ImportanceOfBeingErnest hopefullyextent is now more understandable. Also with a motivation, why this is a bit tricky.

An example would be great, but that's beyond this PR.

@timhoffm
Copy link
MemberAuthor

timhoffm commentedApr 9, 2018
edited
Loading

There's already a PR for an example for origin/extent:#10947

@ImportanceOfBeingErnest
Copy link
Member

Can this be coincidence? That's perfect! I didn't see that at all. It definitely needs to be linked to (in both, origin and extent).

when interpolation is one of: 'sinc', 'lanczos' or 'blackman'.

resample
Undocumented.
Copy link
Member

Choose a reason for hiding this comment

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

can we sort out whatresample does?

Copy link
MemberAuthor

@timhoffmtimhoffmApr 9, 2018
edited
Loading

Choose a reason for hiding this comment

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

I couldn't. I can track down its use to _image_interpolation.h l.987. but that C code doesn't speak to me. It appears to be written by@mdboom. Maybe he could enlighten us?

Copy link
Member

Choose a reason for hiding this comment

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

It is documented inthe resample C wrapper.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Thanks. I've copied the doc. Still I'm not 100% clear what this does:

  • When will resampling (no matter if full or partial) occur.
  • what are the output and the input images?

integer coordinates and their center coordinates range from 0 to
columns-1 horizontally and from 0 to rows-1 vertically.

Note that the meaning of *top* and *bottom* depend on *origin*
Copy link
Member

Choose a reason for hiding this comment

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

This is not quite right, I would say that 'origin' controls how the image is filled in, but not the meaning of 'top' and 'bottom'.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Changed to:

Note that the relation oftop andbottom to data coordinates depends onorigin.

"controls how the image is filled in" is true but does not help for describing the default values.

@tacaswelltacaswell added this to thev3.0 milestoneApr 9, 2018
@timhoffmtimhoffmforce-pushed thedoc-axes-imshow branch 2 times, most recently fromf05bea3 to3f985bdCompareApril 9, 2018 23:06
tacaswell
tacaswell previously requested changesApr 10, 2018
Copy link
Member

@tacaswelltacaswell left a comment

Choose a reason for hiding this comment

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

Still not happy with the changes to theextent docstring. Sorry for being difficult on this.

integer coordinates and their center coordinates range from 0 to
columns-1 horizontally and from 0 to rows-1 vertically.

Note that the relation of *top* and *bottom* to data coordinates
Copy link
Member

Choose a reason for hiding this comment

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

The meaning of 'top' and 'bottom' in data coordinates does not change, what changes is the orientation of the array in the bounding box.

Copy link
Member

Choose a reason for hiding this comment

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

vertical axis.

- If origin is set to "upper", the center of the lower left
pixel is at (0, rows) , this corresponds to an extent
Copy link
Member

Choose a reason for hiding this comment

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

I suggest something like:

  • If origin is set to "upper" then the default extents is (...)
  • If origin is set to "lower" then the default extents is (...)

If the axes auto-scales the limits than this arranges such that the value in X[0, 0] is at (0, 0) in data-coordinates in both cases. In the case of "upper" the (0, 0) is in the upper left and the y-axis increases downward. In the case of "lower", the (0, 0) is in the lower left and the y-axis increases upward.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe this needs a comment "having 'bottom > top' or 'left > right' may invert the drawn image"?

The location, in data-coordinates, of the lower-left and
upper-right corners. If `None`, the image is positioned such that
the pixel centers fall on zero-based (row, column) indices.
upper-right corners.
Copy link
Member

Choose a reason for hiding this comment

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

'... of the image's bounding box'

@timhoffm
Copy link
MemberAuthor

timhoffm commentedApr 10, 2018
edited
Loading

No worries, it‘s this API being difficult, not you.

Maybe we‘re just using the wrong terminology. I would associate top and bottom with the visual appearance. For top > bottom I expect it to be above when I look at the figure. Probably we should talk about the min/max data coordinates, e.g. (xmin, xmax, ymin, ymax). Still have to think this through completely for all cases.

@timhoffmtimhoffmforce-pushed thedoc-axes-imshow branch 2 times, most recently from7b85456 to801226dCompareApril 16, 2018 16:40
@timhoffm
Copy link
MemberAuthor

timhoffm commentedApr 16, 2018
edited
Loading

There's now only a basic mention of the interaction betweenextent andorigin. This is too complicated to be described here in detail. Instead, both parameters should add a link to the example from#10947 once it gets merged.

@ImportanceOfBeingErnest
Copy link
Member

This PR will also fix#10323 once it's merged.

@ImportanceOfBeingErnest
Copy link
Member

@timhoffm can you recheck if this should useM x N orN x M (due tocomment by@efiring)? Potentially#10225 would need to be adjusted as well.

@timhoffm
Copy link
MemberAuthor

Changed to more common notation(M, N).

@timhoffm
Copy link
MemberAuthor

@tacaswell Now that#10947 is merged and linked, I hope that the documentation onextent is right and this PR is good to go.

axes.
aspect : {'equal', 'auto'} or float, optional
Controls the aspect ratio of the axes. The aspect is of particular
relevance for images since it may distort the image, i.e. pixel
Copy link
Member

Choose a reason for hiding this comment

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

i.e. pixels will not be square....

If 'equal', and `extent` is None, changes the axes aspect ratio to
match that of the image. If `extent` is not `None`, the axes
aspect ratio is changed to match that of the extent.
This parameter is just a shortcut for explicitly calling
Copy link
Member

Choose a reason for hiding this comment

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

strike "just"....

`X` must be an array of integers that index directly into
the lookup table of the `cmap`.
norm : `~matplotlib.colors.Normalize`, optional
If scalar data is used, the Normalize instance scales the
Copy link
Member

Choose a reason for hiding this comment

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

"... scalar data are used" (data is usually plural)....

corner of the axes. If None, default to rc `image.origin`.
corner of the axes. The convention 'upper' is typically used for
matrices and images.
Defaults to :rc:`image.origin`.
Copy link
Member

Choose a reason for hiding this comment

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

I have no idea what image.origin defaults to. I think we could/should say what the default of the default is.

The location, in data-coordinates, of the lower-left and
upper-right corners. If `None`, the image is positioned such that
the pixel centers fall on zero-based (row, column) indices.
By default, pixels have a size of (1, 1). They are centered on
Copy link
Member

Choose a reason for hiding this comment

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

I found this first sentence strange.

"""By default the extent of the image is one data unit per pixel, centered on integer co-ordinates. So the horizontal axis ranges from 0 to N-1, and the vertical axes from 0 to M-1. Specifyingextent displays a subset of the data in this co-ordinate system.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

By default the extent of the image is one data unit per pixel, centered on integer co-ordinates.

To me, that sounds even more strange. Extent is a range, which cannot be "one data unit per pixel". Also the extent is not centered, the pixels are.

Specifying extent displays a subset of the data in this co-ordinate system.

That sounds like clipping, which is not true. Extent scales the data.

Copy link
Member

@jklymakjklymakMay 6, 2018
edited
Loading

Choose a reason for hiding this comment

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

Oops my mistake. What a confusing name for the kwarg. Regardless it still isn’t very clear.

How about pixels columns are linearly mapped between extent[0] and extent[3] and rows between ... by default these extents are ....

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Wouldn't bother with columns and linear mapping to explain streching. Here's my current committed version:

The bounding box in data coordinates that the image will fill.
The image is stretched individually along x and y to fill the box.

The default extent is determined by the following conditions.
Pixels have unit size in data coordinates. Their centers are on
integer coordinates, and their center coordinates range from 0 to
columns-1 horizontally and from 0 to rows-1 vertically.

@jklymak
Copy link
Member

I'll give@tacaswell another day to comment if he wants, and then we can merge. Its a big improvement, and can be refined again if necessary.

See also the `filternorm` and `filterrad` parameters.
If `interpolation` is 'none', then no interpolation is performed
See also the *filternorm* and *filterrad* parameters.
If *interpolation* is 'none', then no interpolation is performed
on the Agg, ps and pdf backends. Other backends will fall back to
'nearest'.

Choose a reason for hiding this comment

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

A totally minor suggestion: At this point it could make sense to link to theinterpolation_methods example.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Sure. Added. Revisiting the interpolation docstring revealed that it was not good anyway. So improved that as well.

@jklymakjklymak merged commit9c7e0de intomatplotlib:masterMay 7, 2018
@jklymak
Copy link
Member

Thanks a lot@timhoffm - if anyone still has issues w/ this, we can have a new PR 😉....

timhoffm reacted with hooray emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@QuLogicQuLogicQuLogic left review comments

@ImportanceOfBeingErnestImportanceOfBeingErnestImportanceOfBeingErnest left review comments

@jklymakjklymakjklymak approved these changes

@tacaswelltacaswelltacaswell left review comments

Assignees
No one assigned
Projects
None yet
Milestone
v3.0.0
Development

Successfully merging this pull request may close these issues.

7 participants
@timhoffm@anntzer@ImportanceOfBeingErnest@phobson@jklymak@tacaswell@QuLogic

[8]ページ先頭

©2009-2025 Movatter.jp