Skip to content

Use traditional linestyle shortcuts - #4241

Closed
ycopin wants to merge 1 commit into
matplotlib:masterfrom
ycopin:patch-1
Closed

ycopin wants to merge 1 commit into
matplotlib:masterfrom
ycopin:patch-1

Conversation

@ycopin

@ycopin ycopin commented Mar 18, 2015

Copy link
Copy Markdown

'-'=solid, '--'=dashed, ':'=dotted, '-.'=dashdot. The conversion is done on the fly, no other changes needed in the code.

'-'=solid, '--'=dashed, ':'=dotted, '-.'=dashdot. The conversion is done on the fly, no other changes needed in the code.
@tacaswell tacaswell added this to the proposed next point release milestone Mar 18, 2015
@tacaswell

Copy link
Copy Markdown
Member

What is the motivation for this? The graphics context is a very internal thing so we can be very picky about it's input.

@WeatherGod

Copy link
Copy Markdown
Member

Isn't there already a PR trying to solve this, and it got mired down in
edge cases?

On Wed, Mar 18, 2015 at 9:01 AM, Thomas A Caswell notifications@github.com
wrote:

What is the motivation for this? The graphics context is a very internal
thing so we can be very picky about it's input.

—
Reply to this email directly or view it on GitHub
#4241 (comment)
.

@WeatherGod

Copy link
Copy Markdown
Member

Btw, consistency is the motivation for this. I opened issue #2136 a while
back when I noticed this. And the open PR is #3772

On Wed, Mar 18, 2015 at 9:21 AM, Benjamin Root ben.root@ou.edu wrote:

Isn't there already a PR trying to solve this, and it got mired down in
edge cases?

On Wed, Mar 18, 2015 at 9:01 AM, Thomas A Caswell <
notifications@github.com> wrote:

What is the motivation for this? The graphics context is a very internal
thing so we can be very picky about it's input.

—
Reply to this email directly or view it on GitHub
#4241 (comment)
.

@tacaswell

Copy link
Copy Markdown
Member

@WeatherGod The other PR is taking care of this validation/consistency at the Artist level, not at the GraphicsContext level. We shouldn't do normalization in both places and should do the normalization/validation as close to the user as possible (so that they get an exception at artist creation time, not at draw time).

@WeatherGod

Copy link
Copy Markdown
Member

I agree with that. I just wanted to make sure that we choose this PR for
lack of another solution, or maybe we decide that this is better than the
rather extensive changes shown in the other PR. I like having ducks in a
row.

On Wed, Mar 18, 2015 at 9:32 AM, Thomas A Caswell notifications@github.com
wrote:

@WeatherGod https://github.com/WeatherGod The other PR is taking care
of this validation/consistency at the Artist level, not at the
GraphicsContext level. We shouldn't do normalization in both places and
should do the normalization/validation as close to the user as possible (so
that they get an exception at artist creation time, not at draw time).

—
Reply to this email directly or view it on GitHub
#4241 (comment)
.

@WeatherGod

Copy link
Copy Markdown
Member

err.. not choose

On Wed, Mar 18, 2015 at 9:40 AM, Benjamin Root ben.v.root@gmail.com wrote:

I agree with that. I just wanted to make sure that we choose this PR for
lack of another solution, or maybe we decide that this is better than the
rather extensive changes shown in the other PR. I like having ducks in a
row.

On Wed, Mar 18, 2015 at 9:32 AM, Thomas A Caswell <
notifications@github.com> wrote:

@WeatherGod https://github.com/WeatherGod The other PR is taking care
of this validation/consistency at the Artist level, not at the
GraphicsContext level. We shouldn't do normalization in both places and
should do the normalization/validation as close to the user as possible (so
that they get an exception at artist creation time, not at draw time).

—
Reply to this email directly or view it on GitHub
#4241 (comment)
.

@tacaswell

Copy link
Copy Markdown
Member

I think the other PR is very close. It needs a final review but I have not
had time to get back to it.

On Wed, Mar 18, 2015 at 9:41 AM Benjamin Root notifications@github.com
wrote:

err.. not choose

On Wed, Mar 18, 2015 at 9:40 AM, Benjamin Root ben.v.root@gmail.com
wrote:

I agree with that. I just wanted to make sure that we choose this PR for
lack of another solution, or maybe we decide that this is better than the
rather extensive changes shown in the other PR. I like having ducks in a
row.

On Wed, Mar 18, 2015 at 9:32 AM, Thomas A Caswell <
notifications@github.com> wrote:

@WeatherGod https://github.com/WeatherGod The other PR is taking care
of this validation/consistency at the Artist level, not at the
GraphicsContext level. We shouldn't do normalization in both places and
should do the normalization/validation as close to the user as possible
(so
that they get an exception at artist creation time, not at draw time).

—
Reply to this email directly or view it on GitHub
<
https://github.com/matplotlib/matplotlib/pull/4241#issuecomment-82972555>
.

—
Reply to this email directly or view it on GitHub
#4241 (comment)
.

@ycopin

ycopin commented Mar 18, 2015

Copy link
Copy Markdown
Author

Sorry for not having seen the other PR, which is indeed way more complete (including user-level documentation). Yet I don't understand comment "get an exception at artist creation time, not at draw time". As for now, an invalid linestyle is intercepted in GraphicsContextBase.set_linestyle() in backend_bases.py, not any time sooner (this is how I found it).

@WeatherGod

Copy link
Copy Markdown
Member

Right. The point is that if an exception is thrown at draw time, the user
will have a harder time tracing the problem back to its origin. Whereas if
we raise the exception at the call to plt.plot() or some other plotting
function for invalid values, it would be much easier to debug.

On Wed, Mar 18, 2015 at 9:55 AM, Yannick Copin notifications@github.com
wrote:

Sorry for not having seen the other PR, which is indeed way more complete
(including user-level documentation). Yet I don't understand comment "get
an exception at artist creation time, not at draw time". As for now, an
invalid linestyle is intercepted in GraphicsContextBase.set_linestyle()
in backend_bases.py, not any time sooner (this is how I found it).

—
Reply to this email directly or view it on GitHub
#4241 (comment)
.

@tacaswell

Copy link
Copy Markdown
Member

iirc, one of the things that #3772 does is to push the exceptions up to creation time.

@efiring

efiring commented May 14, 2015

Copy link
Copy Markdown
Member

I think this is superceded by #3772 which has now been merged. If not, please reopen it. In any case, thank you for the contribution.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants