DOC: add tutorial explaining imshow origin and extent #10947

tacaswell · 2018-04-03T04:15:24Z

PR Summary

Sort out how origin and 'extent interact.

attn @efiring

PR Checklist

dstansby

A couple of suggestions for improvement.

dstansby · 2018-04-07T17:53:22Z

tutorials/intermediate/imshow_extent.py

+from matplotlib.gridspec import GridSpec
+
+
+def generate_imshow_demo_grid(auto_limits, extents):


Might be worth having these as keyword arguments? When I read the tutorial I scrolled past this big block of code, and then was confused as to what generate_imshow_demo_grid(True...) did, but I think generate_imshow_demo_grid(auto_limits=True...) would be much better.

dstansby · 2018-04-07T17:55:27Z

tutorials/intermediate/imshow_extent.py

+# ``np.arange(42).reshape(6, 7)`` with varying parameters.
+#
+
+generate_imshow_demo_grid(True, extents[:1])


There isn't a massive change in these images from L --> R, but a clear different from top to bottom. Maybe a good place to use a colourmap that isn't perceptually uniform to illustrate changes in orientation?

At least when I look a these what I see is the direction of highest-lowest vector, changed to a diagonal gradient.

ImportanceOfBeingErnest · 2018-04-09T15:25:21Z

Geat tutorial! I did suggest to have such example around in #10982 without seeing this one being created.

I used triangular shaped data to explain a point I wanted to make in that PR, something like

This would make it easier to grasp at which corner the data is shown compared to the original array. Would this be an option here?

ImportanceOfBeingErnest · 2018-04-09T18:16:55Z

tutorials/intermediate/imshow_extent.py

-
-    d = np.arange(42).reshape(6, 7)
+    x, y = np.ogrid[0:6, 0:7]
+    d = x + y


👍 yes, that's what I meant!

timhoffm · 2018-04-16T17:10:24Z

tutorials/intermediate/imshow_extent.py

+#   ``(left, bottom)`` corner of the bounding box and moving closer to
+#   ``(left, top)`` moves along the ``[:, 0]`` axis of the array to
+#   higher indexed rows and moving towards ``(right, bottom)`` moves you
+#   along the ``[0, :]`` axis of the array to higher indexed columns


Possibly better as a bullet list or table than plain text:

Generally, for an array of shape (M,N), the first index runs along the vertical, the second index runs along the horizontal. origin determines how to the data is filled in the bounding box.

For origin='lower':

[0, 0] is at (left, bottom)

[M, 0] is at (left, top)

[0, N] is at (right, bottom)

[M, N] is at (right, top)

origin='upper' reverses the vertical filling:

[0, 0] is at (left, top)

[M, 0] is at (left, bottom)

[0, N] is at (right, top)

[M, N] is at (right, bottom)

timhoffm · 2018-04-16T20:55:37Z

tutorials/intermediate/imshow_extent.py

+# is ``(-0.5, numcols-0.5, numrows-0.5, -0.5)`` when ``origin ==
+# 'upper'`` and ``(-0.5, numcols-0.5, -0.5, numrows-0.5)`` when ``origin
+# == 'lower'`` which puts the pixel centers on integer positions and the
+# ``[0, 0]`` pixel at ``(0, 0)`` in dataspace.


I think it's simpler to understand if this was a table with colums 'origin == upper', 'origin == lower' and rows

[0, 0] pixel position

default extent

maybe more.

timhoffm · 2018-04-16T21:00:29Z

tutorials/intermediate/imshow_extent.py

+                       'xy': (1, .5)}
+        if extent is None:
+            ax.annotate('None', **text_kwargs)
+            ax.set_title('`extent=`')


Backticks don't work here. We can just leave them out.

man, talk about muscle memory....

timhoffm · 2018-04-16T21:04:24Z

tutorials/intermediate/imshow_extent.py

+from matplotlib.gridspec import GridSpec
+
+
+def generate_imshow_demo_grid(extents, auto_limits):


I find it more natural with a reversed logic that uses explicit_limits=False instead of auto_limits, or alternatively, xlims=None, ylims=None.

timhoffm · 2018-04-16T21:15:31Z

tutorials/intermediate/imshow_extent.py

+
+###############################################################################
+#
+# If we fix the axes limits so ``(0, 0)`` is at the bottom left and


If we fix the axes limits by explicity setting set_xlim / set_ylim, we force a certain orientation of the axes. This can decouple the 'left-right' and 'top-bottom' sense of the image from the orientation on the screen.

In the example below (0, 0) is explicitly put at the bottom left and values increase to up and to the right (from the viewer point of view). We can see that:

timhoffm · 2018-04-16T21:28:27Z

Also it helps to display the indices in the plots:

def index_to_coordinate(index, extent, origin):
    left, right, bottom, top = extent

    # convert from extent to corner pixel centers    
    hshift = 0.5 if left < right else -0.5
    left, right = left + hshift, right - hshift
    vshift = 0.5 if bottom < top else -0.5
    bottom, top = bottom + vshift, top - vshift
    
    # handle vertical fill order
    if origin == 'upper':
        bottom, top = top, bottom
    
    return {
        '0, 0': (left, bottom),
        'M, 0': (left, top),
        '0, N': (right, bottom),
        'M, N': (right, top),
    }[index]

and in the inner for loop:

            for index in ['0, 0', '0, N', 'M, 0', 'M, N']:
                ax.text(*index_to_coordinate(index, extent, origin),
                        index, color='white', ha='center', va='center')

ImportanceOfBeingErnest · 2018-04-16T22:08:00Z

Just mind the notation. In the imshow docs (#10982), arrays are of shape (N,M). ~~This is consistent with widely used conventions.~~ So if updating the coordinates as proposed above, it would make sense to use this convention as well, and not suddenly chage it to (M,N). Independently something seems to be wrong in the above suggestion.

For a tuple a,b one needs to decide if this is a cartesian point (x,y) or an array index [i,j]
In either case, the last coordinate or index should be N-1 and M-1 respectively.

timhoffm · 2018-04-16T23:37:29Z

@ImportanceOfBeingErnest Agreed on all points.

Should use (N,M).
The exact notation would be [N-1, M-1], but there is no way of fitting that in square pixels while maintaining a reasonably small figure size. I could discuss away the square brackets with a sentence: "Values in pixels describe their indices". No good idea how to cope with the -1 issue. One could define them one-smaller by "array of shape (N+1,M+1)". That would be consistent within the example, but other than the original docstring notation (N, M).

timhoffm · 2018-04-18T14:51:05Z

Probably the best solution is using boxes for the indices as well:

where N' = N-1 and M' = M-1

and code:

def get_color(index, data, cmap):
    val = {
        "[0, 0]": data[0, 0],
        "[0, M']": data[0, -1],
        "[N', 0]": data[-1, 0],
        "[N', M']": data[-1, -1],
    }[index]
    return cmap(val / data.max())

            for index in ["[0, 0]", "[0, M']", "[N', 0]", "[N', M']"]:
                ax.text(*index_to_coordinate(index, extent, origin),
                        index, color='white', ha='center', va='center',
                        bbox={'boxstyle': 'square',
                              'facecolor': get_color(index, d, im.get_cmap())}
                        )

tacaswell · 2018-04-18T19:55:35Z

@timhoffm @ImportanceOfBeingErnest feel free to push directly to this branch, I think all of these suggestions are great 👍

Would probably push the index labels in a bit or give them some alpha, they are covering the tick labels and the arrow. The index labels make also make the arrow superfluous.

efiring · 2018-04-27T02:08:37Z

@ImportanceOfBeingErnest Why is the convention (N, M)? Is this only for images? I find it quite confusing--why use reversed-alphabetical order? What about indices--are they (i, j), or (j, i)?

ImportanceOfBeingErnest · 2018-04-27T08:00:44Z

@efiring Good point. For some reason we're using this convention, but upon reflection and googling, I did not find profound evidence for it being "widely used". I was then somehow confirmed by the recently proposed changes to the imshow docs (#10982). (@timhoffm Might this be a typical german thing?)

So restart from scratch: the imshow docs need to decide for one convention (#10323, #10982), this convention needs to be documented (#10225). This guide here needs to use the same convention.

From a bit of googling and also looking at wikipedia it seems (m,n) / (M,N) / M x N could be preferable above (n,m) / (N,M) / N x M.

timhoffm · 2018-04-28T08:51:30Z

You are right. (M, N) is common, e.g.
https://docs.scipy.org/doc/numpy/reference/generated/numpy.linalg.lstsq.html#numpy.linalg.lstsq
http://scikit-image.org/docs/dev/api/skimage.filters.html#skimage.filters.inverse

Changed.

timhoffm · 2018-04-29T10:34:28Z

I think this is good to go.

For reviewers, here's how it looks like:
https://9315-1385122-gh.circle-artifacts.com/0/home/circleci/project/doc/build/html/tutorials/intermediate/imshow_extent.html?highlight=extent

jklymak · 2018-04-29T16:56:49Z

tutorials/intermediate/imshow_extent.py

+           (-0.5, 6.5, 5.5, -0.5),
+           (6.5, -0.5, -0.5, 5.5),
+           (6.5, -0.5, 5.5, -0.5)]
+


This set of examples doesn't really help me because the "extents" is still the same as the extents I'd expect from the underlying data array size. Wouldn't it be more illustrative to go from 0-15 or something? The phrase "...a bounding box in data space" is also mysterious. I think of the matrix as being the data and a bounding box on the data means a clip to me. But what you mean is that the extents are the x and y co-ordinates over which the matrix is placed.

The docs say "The location, in data-coordinates, of the lower-left and upper-right corners" ; I prefer "data-coordinates"

The point is to show how inverting the left vs right or top vs bottom changes the image around. Using different limits would make that a harder apples-to-apples comparison.

yes But it didn’t help me understand what extents actually is on the first place

I’ll admit this flipping the axes seems like the wrong behaviour. I’d have expected the matrix to flip. But I doubt I’ll get that changed here. ;-)

tacaswell · 2018-04-29T19:10:11Z

I really like the changes! Thanks @ImportanceOfBeingErnest and @timhoffm !

I am 👍 on merging this, but don't want to push the button as I have commits in this and can not approve is github things this is my PR.

timhoffm · 2018-05-02T00:03:19Z

Rephrased the section with "bounding box in data space".
Fixed some typos.

Probably this example could be improved even further, but that would require significant restructuring of code and description. I'm pushing this forward to have a reference to point to from the imshow docstring and get my associated PR #10982 approved. As a first step this PR is now good enough for that. Further improvements can be added later.

jklymak · 2018-05-02T15:07:52Z

I merged as a stand-alone improvement. Can be refined upon...

…*extent*

Backport PR #10947 on branch v2.2.2-doc

tacaswell added this to the v2.2-doc milestone Apr 3, 2018

dstansby added the Documentation label Apr 3, 2018

DOC: add tutorial explaining imshow *origin* and *extent*

1124ec9

tacaswell force-pushed the doc_imshow_extent branch from 519c1dc to 1124ec9 Compare April 3, 2018 14:49

dstansby reviewed Apr 7, 2018

View reviewed changes

timhoffm mentioned this pull request Apr 9, 2018

Improve docstring of Axes.imshow #10982

Merged

DOC: tweak helper method kwargs and example data

e461640

ImportanceOfBeingErnest reviewed Apr 9, 2018

View reviewed changes

timhoffm reviewed Apr 16, 2018

View reviewed changes

DOC: add index labels

285c8d2

timhoffm force-pushed the doc_imshow_extent branch from f1b481b to d1e307a Compare April 28, 2018 08:43

timhoffm added the status: needs review label Apr 29, 2018

jklymak reviewed Apr 29, 2018

View reviewed changes

timhoffm force-pushed the doc_imshow_extent branch from d1e307a to dcb18e0 Compare May 1, 2018 23:52

DOC: Rewrite parts of imshow origin/extent handling

206c3ca

timhoffm force-pushed the doc_imshow_extent branch from dcb18e0 to 206c3ca Compare May 2, 2018 00:02

jklymak merged commit e22a16a into matplotlib:master May 2, 2018

lumberbot-app bot pushed a commit that referenced this pull request May 2, 2018

Backport PR #10947: DOC: add tutorial explaining imshow *origin* and …

f4ac6fb

…*extent*

lumberbot-app bot mentioned this pull request May 2, 2018

Backport PR #10947 on branch v2.2.2-doc #11159

Merged

jklymak added a commit that referenced this pull request May 2, 2018

Merge pull request #11159 from matplotlib/auto-backport-of-pr-10947

7f35bc9

Backport PR #10947 on branch v2.2.2-doc

tacaswell deleted the doc_imshow_extent branch May 3, 2018 12:49

QuLogic removed the status: needs review label Jul 5, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: add tutorial explaining imshow origin and extent #10947

DOC: add tutorial explaining imshow origin and extent #10947

tacaswell commented Apr 3, 2018

dstansby left a comment

dstansby Apr 7, 2018

dstansby Apr 7, 2018

tacaswell Apr 9, 2018

ImportanceOfBeingErnest commented Apr 9, 2018

ImportanceOfBeingErnest Apr 9, 2018

timhoffm Apr 16, 2018 •

edited

Loading

timhoffm Apr 16, 2018

timhoffm Apr 16, 2018

tacaswell Apr 18, 2018

timhoffm Apr 16, 2018

timhoffm Apr 16, 2018

timhoffm commented Apr 16, 2018 •

edited

Loading

ImportanceOfBeingErnest commented Apr 16, 2018 •

edited

Loading

timhoffm commented Apr 16, 2018

timhoffm commented Apr 18, 2018 •

edited

Loading

tacaswell commented Apr 18, 2018

efiring commented Apr 27, 2018

ImportanceOfBeingErnest commented Apr 27, 2018

timhoffm commented Apr 28, 2018

timhoffm commented Apr 29, 2018

jklymak Apr 29, 2018

tacaswell Apr 29, 2018

jklymak Apr 29, 2018

tacaswell commented Apr 29, 2018

timhoffm commented May 2, 2018

jklymak commented May 2, 2018

		from matplotlib.gridspec import GridSpec


		def generate_imshow_demo_grid(auto_limits, extents):

		from matplotlib.gridspec import GridSpec


		def generate_imshow_demo_grid(extents, auto_limits):

DOC: add tutorial explaining imshow *origin* and *extent* #10947

DOC: add tutorial explaining imshow *origin* and *extent* #10947

Conversation

tacaswell commented Apr 3, 2018

PR Summary

PR Checklist

dstansby left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ImportanceOfBeingErnest commented Apr 9, 2018

Choose a reason for hiding this comment

timhoffm Apr 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timhoffm commented Apr 16, 2018 • edited Loading

ImportanceOfBeingErnest commented Apr 16, 2018 • edited Loading

timhoffm commented Apr 16, 2018

timhoffm commented Apr 18, 2018 • edited Loading

tacaswell commented Apr 18, 2018

efiring commented Apr 27, 2018

ImportanceOfBeingErnest commented Apr 27, 2018

timhoffm commented Apr 28, 2018

timhoffm commented Apr 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tacaswell commented Apr 29, 2018

timhoffm commented May 2, 2018

jklymak commented May 2, 2018

DOC: add tutorial explaining imshow origin and extent #10947

DOC: add tutorial explaining imshow origin and extent #10947

timhoffm Apr 16, 2018 •

edited

Loading

timhoffm commented Apr 16, 2018 •

edited

Loading

ImportanceOfBeingErnest commented Apr 16, 2018 •

edited

Loading

timhoffm commented Apr 18, 2018 •

edited

Loading