FIX: set_url() without effect in the plot for instances of Tick #9696
Conversation
In the class Tick, the url attribute is not passed to any of the objects that are to be drawn. Suggested solution is to override the set_url() method in Tick to transfer the url attribute content to the tick labels, that are to be drawn.
|
Why are tests N/A for this? If it had a test, this funtionality wouldn't break in the future... |
tacaswell
left a comment
There was a problem hiding this comment.
Please add the super call to not remove any existing functionality.
| Set the url of label1 and label2 | ||
| ACCEPTS: str | ||
| """ |
There was a problem hiding this comment.
Can you also call super(self, Tick).set_label(s)?
There was a problem hiding this comment.
Hi Thomas, if you meant to add a call to super(Tick, self).set_url(s), I just added it, if not, I don't get it, sorry.
There was a problem hiding this comment.
Travis is failing with that addition with some ImageComparisonFailure: images not close errors. When calling the parent Artist's set_url() method it just does self._url = url! Amazing.
There was a problem hiding this comment.
That is what I meant, thanks.
Can you reproduce the failures locally?
|
How can we add a test for this? Can we parse the XML that is svg well using only our current dependencies to verify that the url is there? |
|
@tacaswell, I was only thinking a test could be devised to make sure the url was attached to the tick. Whether its embeded or not is another issue. |
|
For testing one can probably extract the node with the right XPath query (https://docs.python.org/3.6/library/xml.etree.elementtree.html#xpath-support) and do the right assert on it... Or more simply one can just compare the resulting svg with manually setting the url onto the label (tick.label1.set_url(...); tick.label2.set_url(...)); they should be exactly identical. |
|
I've incorporated this into #17338 and added a test. |
|
Many thanks, @QuLogic, and sorry, it seems that this fell off my radar at some point :( |
|
@khyox It fell off ours too 😞 |
PR Summary
In the class Tick, the url attribute is not passed to any of the objects that are to be drawn, so it is useless, as detailed in issue #9695. Suggested solution is to override the
set_url()method in Tick to transfer the url attribute content to the tick labels, that are to be drawn.Fixes issue #9695.
PR Checklist