/matplotlib

Remove explicit children invalidation in update_position method #6692

Merged
merged 1 commit into from Jul 12, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
  2.1 (next point r...
3 participants
@Kojoley @tacaswell @mdboom
@Kojoley
Member

Kojoley commented Jul 5, 2016

This invalidation actually does nothing as the Line2D does not have nor use the _invalid field. There are _invalidx and _invalidy fields for this purpose, but them are private members. Moreover, the set_xdata and set_ydata methods internally invalidate the state (what is more reasonable
then an external invalidation).

mdboom added the needs_review label Jul 5, 2016

@tacaswell tacaswell and 1 other commented on an outdated diff Jul 5, 2016

View full outdated diff
lib/matplotlib/axis.py
@@ -446,15 +446,8 @@ def _get_gridline(self):
return l
- def update_position(self, loc):
- 'Set the location of tick in data coords with scalar *loc*'
- x = loc
-
- nonlinear = (hasattr(self.axes, 'yaxis') and
- self.axes.yaxis.get_scale() != 'linear' or
- hasattr(self.axes, 'xaxis') and
- self.axes.xaxis.get_scale() != 'linear')
-
+ def update_position(self, x):
@tacaswell

tacaswell Jul 5, 2016

Owner

Please do not change the arg names, it will break anyone who is unpacking dictionaries in to this method.

@Kojoley

Kojoley Jul 5, 2016

Member

Oh, sorry, I though that kwargs cannot be unpacked into positional arguments.

tacaswell added this to the 2.1 (next point release) milestone Jul 5, 2016

@tacaswell
Owner

tacaswell commented Jul 5, 2016

It looks like these code blocks originate in 80cc9aa (in 2007) from @mdboom .

The commit message says that this was a performance related change.
@tacaswell
Owner

tacaswell commented Jul 5, 2016

appveyor failure is connectivity to anaconda.org issue
@Kojoley
Member

Kojoley commented Jul 5, 2016

I will make a performance test then, but the test suit runs the same amount of time with this change. And as I have mentioned in the commit message Line2D._invalid is unexisting field so it would be surprise if it actually does something.
@tacaswell
Owner

tacaswell commented Jul 5, 2016

I suspect that (given the age) it was using some internal details that have moved under it. Don't worry about adding a performance test (unless @mdboom disagrees).

I think this is otherwise 👍 .
@Kojoley Kojoley Remove explicit children invalidation in update_position method 
This invalidation actually does nothing as the `Line2D` does not have nor use
the `_invalid` field. There are `_invalidx` and `_invalidy` fields for this
purpose, but them are private members. Moreover, the `set_xdata` and
`set_ydata` methods internally invalidate the state (what is more reasonable
then an external invalidation).

Some checks were not successful

1 errored, 1 failing, and 1 successful checks
continuous-integration/travis-ci/pr — The Travis CI build could not complete due to an error
Details
coverage/coveralls — Coverage decreased (-0.01%) to 70.265%
Details
continuous-integration/appveyor/pr — AppVeyor build succeeded
Details
0847006

@tacaswell tacaswell merged commit a578a7a into matplotlib:master Jul 12, 2016
1 of 3 checks passed

1 of 3 checks passed

coverage/coveralls Coverage decreased (-0.01%) to 70.265%
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

tacaswell removed the needs_review label Jul 12, 2016

Kojoley deleted the Kojoley:cleanup-axis-update_position branch Jul 12, 2016

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