Fixed GraphicsContextBase linestyle getter #7730

Merged
merged 1 commit into from Jan 13, 2017

Conversation

Projects
None yet
5 participants
Member

Kojoley commented Jan 2, 2017

Closes #3354

@Kojoley Kojoley Fixed GraphicsContextBase linestyle getter
0980bc5

Kojoley added this to the 2.1 (next point release) milestone Jan 2, 2017

Owner

tacaswell commented Jan 3, 2017

For reference, this has been like this since e34a333 and has no previous history.

We also do not appear to ever use it internally. All of our backends use get_dashes instead which (mostly) ensures that the dash pattern is the same across all backends (and enables us to scale the pattern with line width).

This needs an API changes note (there may be third-party backends working around this), but given that the current API is obviously problematic (and no one has complained) I do not think it needs a deprecation cycle.

Member

Kojoley commented Jan 3, 2017

GraphicsContextBase.get_linestyle is not used anywhere in mpl code.
GraphicsContextBase._linestyle is used only in GraphicsContextBase.copy_properties and it sets the variable DIRECTLY.
GraphicsContextBase.set_linestyle is used only in one place and overridden only in wx backend.
There is a TODO entry in pdf backend about _linestyle from 2006 year and I am not sure it makes any sense now.

So I assume it is safe to fix this without deprecation (or even to remove the property).

Contributor

anntzer commented Jan 8, 2017

If we're going to break backcompat, let's just remove the property?

tacaswell closed this Jan 9, 2017

tacaswell reopened this Jan 9, 2017

Owner

tacaswell commented Jan 9, 2017

power cycled to run CI with fixes from master branch to other tests.

codecov-io commented Jan 9, 2017 edited

Current coverage is 62.52% (diff: 100%)

Merging #7730 into master will increase coverage by 0.40%

@@             master      #7730   diff @@
==========================================
  Files           174        174          
  Lines         56028      56703   +675   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          34805      35453   +648   
- Misses        21223      21250    +27   
  Partials          0          0          

Powered by Codecov. Last update 8279f64...0980bc5

anntzer referenced this pull request Jan 9, 2017

Merged

More code removal #7771

Contributor

NelleV commented Jan 13, 2017

LGTM 👍
Thanks @Kojoley

@NelleV NelleV merged commit 26a30ce into matplotlib:master Jan 13, 2017

5 checks passed

codecov/patch 100% of diff hit (target 62.12%)
Details
codecov/project 62.52% (+0.40%) compared to 8279f64
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.004%) to 62.117%
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment