Skip to content

_axes.py linestyle_map unused #7162

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
Carreau opened this issue Sep 22, 2016 · 11 comments
Closed

_axes.py linestyle_map unused #7162

Carreau opened this issue Sep 22, 2016 · 11 comments
Labels
Difficulty: Easy https://matplotlib.org/devdocs/devel/contribute.html#good-first-issues
Milestone
v2.0.0

Comments

@Carreau
Copy link
Contributor

Carreau commented Sep 22, 2016

This thing seem to be unused to me:

        linestyle_map = {
            'solid': '-',
            'dashed': '--',
            'dashdot': '-.',
            'dotted': ':'
        }

I'm not sure if this is on purpose, from git log it seem like it used to be to allow what is now final_boxprops to support the "dotted", "solid" keyword somewhere (not sure where).
If it's not usefull anymore it might be removed I guess.
@Kojoley
Copy link
Member

Kojoley commented Sep 22, 2016

Would you like to open a PR?

@Kojoley Kojoley added the Difficulty: Easy https://matplotlib.org/devdocs/devel/contribute.html#good-first-issues label Sep 22, 2016
@WeatherGod
Copy link
Member

I have a vague recollection of commenting on this in a PR in the past year.
I just wish I could remember what the response was. I suspect the
explanation is in a comment for a PR to fix up linestyle name handling
(maybe the whole None/'none' mess?).

On Thu, Sep 22, 2016 at 1:36 PM, Nikita Kniazev notifications@github.com
wrote:

Would you like to open a PR?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#7162 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AARy-GF2bqi67d5dbaRkrb2NHiRAjKC3ks5qsryMgaJpZM4KEJAj
.

@Carreau
Copy link
Contributor Author

Carreau commented Sep 22, 2016

Would you like to open a PR?

Removing sure, if removing is the thing to do; but I'm not sure what it was supposed to handle, so if there is something else to fix I'm not sure if I can make a PR before knowing what the expected behavior should be.

@WeatherGod
Copy link
Member

I wish there was a way to search for all review comments on a given set of
code lines. Heck... is there any method for searching review comments?

On Thu, Sep 22, 2016 at 1:51 PM, Matthias Bussonnier <
notifications@github.com> wrote:

Would you like to open a PR?

Removing sure, if removing is the thing to do; but I'm not sure what it
was supposed to handle, so if there is something else to fix I'm not sure
if I can make a PR before knowing what the expected behavior should be.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#7162 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AARy-Hb1Mn9Z-Z3nW2tS2WjFUDi5xUOgks5qssA6gaJpZM4KEJAj
.

@Kojoley
Copy link
Member

Kojoley commented Sep 22, 2016

Was added with 65a686c and then all related code was removed in #3165

@tacaswell
Copy link
Member

attn @anntzer

@tacaswell
Copy link
Member

Err, never mind, wrong set of mappings between strings and line styles.

@tacaswell tacaswell added this to the 2.0.1 (next bug fix release) milestone Sep 23, 2016
@anntzer
Copy link
Contributor

anntzer commented Sep 23, 2016

Yup, never touched this part of the code :)

@Carreau
Copy link
Contributor Author

Carreau commented Sep 23, 2016

I'm tempted to say, if it was added on the 2.x cycle it's safe to remove right ?

@Kojoley
Copy link
Member

Kojoley commented Sep 23, 2016

It is a local variable, and I have shown above that all related code was removed, so it is definitely safe to remove.

Carreau added a commit to Carreau/matplotlib that referenced this issue Sep 23, 2016
@Carreau
Remove unused variable.
1c084ff 
This was added during 2.0-dev cycle, all it uses were removed.
So it is not useful to keep around anymore, and is local anyway.

Closes matplotlib#7162
@Carreau Carreau mentioned this issue Sep 23, 2016
Merged
@Carreau
Copy link
Contributor Author

Carreau commented Sep 23, 2016

Pr done: #7169

@QuLogic QuLogic modified the milestones: 2.0 (style change major release), 2.0.1 (next bug fix release) Dec 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Difficulty: Easy https://matplotlib.org/devdocs/devel/contribute.html#good-first-issues
Projects
None yet
Milestone
v2.0.0
Development

No branches or pull requests
6 participants
@tacaswell @WeatherGod @QuLogic @Carreau @anntzer @Kojoley