Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
More code removal#7771
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
More code removal#7771
Uh oh!
There was an error while loading.Please reload this page.
Conversation
81236ff to58f81f6Comparecodecov-io commentedJan 10, 2017 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Current coverage is 62.11% (diff: 42.85%)@@ master #7771 diff @@========================================== Files 174 174 Lines 56028 55984 -44 Methods 0 0 Messages 0 0 Branches 0 0 ==========================================- Hits 34803 34777 -26+ Misses 21225 21207 -18 Partials 0 0
|
lib/matplotlib/backend_bases.py Outdated
| @@ -912,13 +910,6 @@ def get_joinstyle(self): | |||
| """ | |||
| return self._joinstyle | |||
| def get_linestyle(self, style): | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
These need to go through a deprecation cycle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
In#7730 (comment), you were fine changing the signature ofget_linestyle (which would be a breaking change for anyone (hem) relying on this method) without a deprecation cycle, is it clear that removing the method is any different?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That is a fair point. Currently considering my inconsistencies 😜 .
| @@ -1024,26 +1024,3 @@ def get_paths_extents(paths, transforms=[]): | |||
| raise ValueError("No paths provided") | |||
| return Bbox.from_extents(*_path.get_path_collection_extents( | |||
| Affine2D(), paths, transforms, [], Affine2D())) | |||
| def _define_deprecated_functions(ns): | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
👍
| funcname = self._lineStyles.get(self._linestyle, '_draw_nothing') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This will now raise on non-validself._linestyle. Do the git logs provide any clue why was this coded this way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
set_linestyle (the class here inherits it from lines.Line2D) now has extensive validation before setting_linestyle, which means invalid_linestyle should never happen. The same code inlines was committed by jdh as far back as 2004, whereas the history ofset_linestyle is a bit difficult to unwind but I'd guess the checking came later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
👍
| @@ -1242,26 +1241,6 @@ def set_dashes(self, seq): | |||
| else: | |||
| self.set_linestyle((0, seq)) | |||
| def _draw_solid(self, renderer, gc, path, trans): | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I do not think we want to remove setting these with out a deprecation cycle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
butbutbut they're private :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I do not mind removing the methods, it the inner call togc.set_linestyle that I am worried about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
(I meant to click on the line below and apparently missed).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
The whole point is that graphics contexts don't have that property anymore.
| self.set_linewidth(1) | ||
| self._pen.SetStyle(self._style) | ||
| self.gfx_ctx.SetPen(self._pen) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Does this pen setting just get replaced elsewhere, or do we never use a pen?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We never useset_linestyle. Pens are set in other parts of the code though.
| @@ -409,8 +409,6 @@ class GraphicsContextWx(GraphicsContextBase): | |||
| 'miter': wx.JOIN_MITER, | |||
| 'round': wx.JOIN_ROUND} | |||
| _dashd_wx = wxc.dashd_wx | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Should we deprecatedashd_wx as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
There's a bunch of other entries in wx_compat.py which are not used (e.g. StockCursor) so I think that would belong to a (separate) cleanup of wx_compat.
I am still concerned about pulling the (unused by us) public method off of the |
Also simplify linestyle handling by Line2D.
Turned the removal into a deprecation. |
Ping@tacaswell? |
Alternative to#7730 (see#7730 (comment) in particular).