Repository navigation
Closed
Labels
Difficulty: Mediumhttps://matplotlib.org/devdocs/devel/contribute.html#good-first-issueshttps://matplotlib.org/devdocs/devel/contribute.html#good-first-issuestopic: mpl_toolkit
Milestone
Description
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 Test case is below.obspy3-runtests -d obspy.station.tests.test_station.StationTest.test_response_plot.
With mpl 1.3.1, the plot is as so:

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

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
Metadata
Metadata
Assignees
Labels
Difficulty: Mediumhttps://matplotlib.org/devdocs/devel/contribute.html#good-first-issueshttps://matplotlib.org/devdocs/devel/contribute.html#good-first-issuestopic: mpl_toolkit

Activity
tacaswell commentedon Jan 19, 2015
man, that commit message is embarrassing ...
tacaswell commentedon Jan 19, 2015
I suspect something subtle got broken when annotations changed from a 'has-a' to 'is-a'
Textobject.My guess is that something changed about where the base of the arrow is calculated and poor interaction with the
FancyArrowPatchobject that gets created.I am some what more distressed by the extra blue lines around the vertical blue dashed lines.
tacaswell commentedon Jan 19, 2015
in
_update_positoin_xytextthere are a bunch of calls toget_window_extentwhich is where I suspect the issue is (as the reported extent of the annotation object may have changed.(mostly notes to my self)
WeatherGod commentedon Jan 19, 2015
"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]
tacaswell commentedon Jan 19, 2015
@WeatherGod How recently?
WeatherGod commentedon Jan 19, 2015
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:
QuLogic commentedon Jan 20, 2015
For a simpler example, I took the
annotation_demo.pyexample and changed thearrowpropsto match the one in ObsPy.Here is 1.3.1:

Here is 1.4.2:

And the 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.QuLogic commentedon Jan 20, 2015
Yea, that's odd too. I will try to see where those lines originate.
QuLogic commentedon Jan 21, 2015
I honestly don't know where these are from, and they stopped showing up, so I wouldn't worry about them.
tacaswell commentedon Jan 21, 2015
this seems to be fixed by #4019
QuLogic commentedon Jan 21, 2015
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:
With 1.3.1:



and with 1.4.2:
Diff:
If you remove the
bbox, they work the same.tacaswell commentedon Jan 31, 2015
Digging into this I still have no idea what the problem is. My suspicion is that something I changed had a crucial side effect...
8 remaining items
tacaswell commentedon Feb 7, 2015
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.
tacaswell commentedon Jul 22, 2015
@QuLogic Can you check your examples with current master, I think #4178 fixes part of the issues.
QuLogic commentedon Jul 22, 2015
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)?
tacaswell commentedon Jul 22, 2015
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?
QuLogic commentedon Jul 22, 2015
From running it on my system, using the original
mpl131.pngI generated a while back for reference (which is byte-for-byte the same as the one here, it seems.)efiring commentedon Jul 22, 2015
On 2015/07/22 10:47 AM, Thomas A Caswell wrote:
That looks to me like a difference in rendered width, not in position.
QuLogic commentedon Jul 22, 2015
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
tacaswell commentedon Jul 31, 2015
@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?
Revert " Merge pull request matplotlib#4019 from myshen/annot_neg_coo…
tacaswell commentedon Aug 6, 2015
Closing this as 'good enough'.
@QuLogic Please ping to have this re-opened if you disagree.