Skip to content

Arrow annotations behave differently between 1.3.1 and 1.4.2 #4012

Closed

Description

@QuLogic

Sorry, here's another one without a simplified test case. I will try and get to that tomorrow. For this, you will need matplotlib 1.4.2 and the development version of ObsPy. Then run obspy3-runtests -d obspy.station.tests.test_station.StationTest.test_response_plot. Test case is below.

With mpl 1.3.1, the plot is as so:
station_response

With mpl 1.4.2 (and master), the plot is as so:
station_response

station_response-failed-diff

You can see that the arrows are thinner and shifted a bit.

I believe the relevant bit of code is the following, but I will try to simplify it later:

            trans_above = blended_transform_factory(ax1.transData,
                                                    ax1.transAxes)
            trans_right = blended_transform_factory(ax1.transAxes,
                                                    ax1.transData)
            arrowprops = dict(
                arrowstyle="wedge,tail_width=1.4,shrink_factor=0.8", fc=color)
            bbox = dict(boxstyle="round", fc="w")
            ax1.annotate("%.1g" % self.instrument_sensitivity.frequency,
                         (self.instrument_sensitivity.frequency, 1.0),
                         xytext=(self.instrument_sensitivity.frequency, 1.1),
                         xycoords=trans_above, textcoords=trans_above,
                         ha="center", va="bottom",
                         arrowprops=arrowprops, bbox=bbox)
            ax1.annotate("%.1e" % self.instrument_sensitivity.value,
                         (1.0, self.instrument_sensitivity.value),
                         xytext=(1.05, self.instrument_sensitivity.value),
                         xycoords=trans_right, textcoords=trans_right,
                         ha="left", va="center",
                         arrowprops=arrowprops, bbox=bbox)

I did a bisect and the bad commit was:

3363b125715724ad89d4b33cbddb23e31e7277d5 is the first bad commit
commit 3363b125715724ad89d4b33cbddb23e31e7277d5
Author: Thomas A Caswell <[email protected]>
Date:   Tue Aug 27 17:07:58 2013 -0500

    re factored `_AnnoationBase` to push keeping track of there the
    secondary object is to the sub-classes.

    Added properties to maintain back-compatibility

    added properties to replace xytext and textcoords (xyann, anncoords)

    properly deprecated `textcoord` and `xytext` in favor of `anncoords`
    and `xyann`.

:040000 040000 f17d8e1a70232471435f96b1faa8550bc697ec32 567d4a0a86bb3d1fb7ec55dc0fa98e86c1dec71a M  doc
:040000 040000 34339e9d0d9f49477908fc3c3227b1b7b55735b4 4dc225e1358d422aa86f9361672c36e127955bee M  lib

Activity

  1. tacaswell commented on Jan 19, 2015

    @tacaswell
    Member

    man, that commit message is embarrassing ...

  2. added this to the v1.4.3 milestone on Jan 19, 2015
  3. self-assigned this
    on Jan 19, 2015
  4. tacaswell commented on Jan 19, 2015

    @tacaswell
    Member

    I suspect something subtle got broken when annotations changed from a 'has-a' to 'is-a' Text object.

    My guess is that something changed about where the base of the arrow is calculated and poor interaction with the FancyArrowPatch object that gets created.

    I am some what more distressed by the extra blue lines around the vertical blue dashed lines.

  5. tacaswell commented on Jan 19, 2015

    @tacaswell
    Member

    in _update_positoin_xytext there are a bunch of calls to get_window_extent which is where I suspect the issue is (as the reported extent of the annotation object may have changed.

    (mostly notes to my self)

  6. WeatherGod commented on Jan 19, 2015

    @WeatherGod
    Member

    "something changed about where the base of the arrow is calculated"

    This sounds familiar. I think we fixed a real bug wrt how the arrow was
    positioned awhile back.

    On Mon, Jan 19, 2015 at 11:49 AM, Thomas A Caswell <[email protected]

    wrote:

    I suspect something subtle got broken when annotations changed from a
    'has-a' to 'is-a' Text object.

    My guess is that something changed about where the base of the arrow is
    calculated and poor interaction with the FancyArrowPatch object that gets
    created.

    I am some what more distressed by the extra blue lines around the vertical
    blue dashed lines.

    —
    Reply to this email directly or view it on GitHub
    #4012 (comment)
    .

  7. tacaswell commented on Jan 19, 2015

    @tacaswell
    Member

    @WeatherGod How recently?

  8. WeatherGod commented on Jan 19, 2015

    @WeatherGod
    Member

    Perhaps this (or one of the ones referenced in it)?
    #566
    But, for some reason, I am thinking of something more recently (like, a bug
    fixed for 1.3.x)

    On Mon, Jan 19, 2015 at 2:57 PM, Thomas A Caswell [email protected]
    wrote:

    @WeatherGod https://github.com/WeatherGod How recently?

    —
    Reply to this email directly or view it on GitHub
    #4012 (comment)
    .

  9. QuLogic commented on Jan 20, 2015

    @QuLogic
    MemberAuthor

    For a simpler example, I took the annotation_demo.py example and changed the arrowprops to match the one in ObsPy.

    import numpy as np
    import matplotlib.pyplot as plt
    from matplotlib.patches import Ellipse
    
    
    fig = plt.figure()
    ax = fig.add_subplot(111, autoscale_on=False, xlim=(-1,5), ylim=(-3,5))
    
    t = np.arange(0.0, 5.0, 0.01)
    s = np.cos(2 * np.pi * t)
    ax.plot(t, s, lw=3, color='purple')
    
    arrowprops = dict(arrowstyle="wedge,tail_width=1.4,shrink_factor=0.8")
    
    ax.annotate('axes center', xy=(.5, .5),  xycoords='axes fraction',
                horizontalalignment='center', verticalalignment='center',
                arrowprops=arrowprops)
    
    ax.annotate('pixels', xy=(20, 20),  xycoords='figure pixels',
                arrowprops=arrowprops)
    
    ax.annotate('points', xy=(100, 300),  xycoords='figure points',
                arrowprops=arrowprops)
    
    ax.annotate('offset', xy=(1, 1),  xycoords='data',
                xytext=(-15, 10), textcoords='offset points',
                horizontalalignment='right', verticalalignment='bottom',
                arrowprops=arrowprops)
    
    ax.annotate('local max', xy=(3, 1),  xycoords='data',
                xytext=(0.8, 0.95), textcoords='axes fraction',
                horizontalalignment='right', verticalalignment='top',
                arrowprops=arrowprops)
    
    ax.annotate('a fractional title', xy=(.025, .975),
                xycoords='figure fraction',
                horizontalalignment='left', verticalalignment='top',
                fontsize=20,
                arrowprops=arrowprops)
    
    ax.annotate('bottom right (points)', xy=(-10, 10),
                xycoords='axes points',
                horizontalalignment='right', verticalalignment='bottom',
                fontsize=20,
                arrowprops=arrowprops)
    
    plt.show()

    Here is 1.3.1:
    mpl131

    Here is 1.4.2:
    mpl142

    And the diff:
    diff

    I'm not sure if this adequately replicates the above issue, but it does point to xycoords='axes points' being broken in some way.

  10. QuLogic commented on Jan 20, 2015

    @QuLogic
    MemberAuthor

    I am some what more distressed by the extra blue lines around the vertical blue dashed lines.

    Yea, that's odd too. I will try to see where those lines originate.

  11. QuLogic commented on Jan 21, 2015

    @QuLogic
    MemberAuthor

    I am some what more distressed by the extra blue lines around the vertical blue dashed lines.

    Yea, that's odd too. I will try to see where those lines originate.

    I honestly don't know where these are from, and they stopped showing up, so I wouldn't worry about them.

  12. tacaswell commented on Jan 21, 2015

    @tacaswell
    Member

    this seems to be fixed by #4019

  13. QuLogic commented on Jan 21, 2015

    @QuLogic
    MemberAuthor

    I'm sorry, it seems that simple example exposes a different bug. Here is a test case that should only trigger the bug in the first post:

    import numpy as np
    import matplotlib.pyplot as plt
    from matplotlib.transforms import blended_transform_factory
    
    fig = plt.figure()
    ax = fig.add_subplot(111, autoscale_on=False, xlim=(-1,5), ylim=(-3,5))
    
    t = np.arange(0.0, 5.0, 0.01)
    s = np.cos(2 * np.pi * t)
    ax.plot(t, s, lw=3, color='purple')
    
    trans_above = blended_transform_factory(ax.transData,
                                            ax.transAxes)
    trans_right = blended_transform_factory(ax.transAxes,
                                            ax.transData)
    
    bbox = dict(boxstyle="round", fc="w")
    
    arrowprops = dict(arrowstyle="wedge,tail_width=1.4,shrink_factor=0.8")
    
    ax.annotate("0.1",
                (3, 1.0),
                xytext=(3, 1.1),
                xycoords=trans_above, textcoords=trans_above,
                ha="center", va="bottom",
                arrowprops=arrowprops, bbox=bbox)
    
    ax.annotate("0.2",
                (1.0, 0.75),
                xytext=(1.05, 0.75),
                xycoords=trans_right, textcoords=trans_right,
                ha="left", va="center",
                arrowprops=arrowprops, bbox=bbox)
    
    plt.show()

    With 1.3.1:
    mpl131
    and with 1.4.2:
    mpl142
    Diff:
    diff

    If you remove the bbox, they work the same.

  14. added
    Release criticalFor bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
    on Jan 31, 2015
  15. tacaswell commented on Jan 31, 2015

    @tacaswell
    Member

    Digging into this I still have no idea what the problem is. My suspicion is that something I changed had a crucial side effect...

  16. 8 remaining items

  17. tacaswell commented on Feb 7, 2015

    @tacaswell
    Member

    attn @leejjoon Can you point me in the right direction on this? I think something in getting out of sync in updating where the center of the text is with the bounding box.

  18. modified the milestones: , on Jul 22, 2015
  19. tacaswell commented on Jul 22, 2015

    @tacaswell
    Member

    @QuLogic Can you check your examples with current master, I think #4178 fixes part of the issues.

    so

  20. QuLogic commented on Jul 22, 2015

    @QuLogic
    MemberAuthor

    It's pretty close now. There's a slight difference in the box, but that may be due to font changes (the tick labels have also shifted a little)?

    mpl-master-diff

  21. tacaswell commented on Jul 22, 2015

    @tacaswell
    Member

    I am more concerned about the shift in the plotted line. Did you make the diff from my png, or from running it on your system?

  22. QuLogic commented on Jul 22, 2015

    @QuLogic
    MemberAuthor

    From running it on my system, using the original mpl131.png I generated a while back for reference (which is byte-for-byte the same as the one here, it seems.)

  23. efiring commented on Jul 22, 2015

    @efiring
    Member

    On 2015/07/22 10:47 AM, Thomas A Caswell wrote:

    I am more concerned about the shift in the plotted line.

    That looks to me like a difference in rendered width, not in position.

  24. QuLogic commented on Jul 22, 2015

    @QuLogic
    MemberAuthor

    If you're really interested in the shift, the bisect was inconclusive:

    There are only 'skip'ped commits left to test.
    The first bad commit could be any of:
    4a56cda
    cfbf8d5
    339537d
    8dae818
    6e19dac
    a225968
    697b3ef
    c5ab747
    5204635
    af578f5
    0624664
    6d07179
    ba40160
    d862c47
    d8c1d0d
    We cannot bisect more!
    bisect run cannot continue any more

  25. tacaswell commented on Jul 31, 2015

    @tacaswell
    Member

    @QuLogic That is inside of the massive re-work of how we expose Agg up to python.

    We have also done some changes to how white space around fonts is handled.

    @mdboom I am inclined to call this good enough?

  26. added a commit that references this issue on Jul 31, 2015
  27. tacaswell commented on Aug 6, 2015

    @tacaswell
    Member

    Closing this as 'good enough'.

    @QuLogic Please ping to have this re-opened if you disagree.

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

Metadata

Metadata

Assignees

Labels

Difficulty: Mediumhttps://matplotlib.org/devdocs/devel/contribute.html#good-first-issuestopic: mpl_toolkit

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions