-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
DOC: missing API docs from #8032 #8285
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DOC: missing API docs from #8032 #8285
Conversation
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.
A few comments.
Additionally, I wonder if it would be useful to precise the PR that did this? (I know some other projects do this in their changelogs but I do not know if Matplotlib does)
line width is smaller than the effective pixel size, this may result | ||
in dashed lines turning into solid gray lines. This also required | ||
slightly tweaking the default patterns for '--', ':', and '.-' so that | ||
with the default line-width the final patterns would not change. |
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.
line-width <- line width? (to be consistent with the other utterances of the expression).
Beside, it is not totally true that the new default patterns are exactly the same as before (e.g. the dotted pattern that is not a 50 %-50 % pattern anymore). Maybe the sentence should just state that the default patterns were tweaked to remain similar with the default lw value as well as to avoid “cluttering” with line widths thiner than the previous clipping boundary.
slightly tweaking the default patterns for '--', ':', and '.-' so that | ||
with the default line-width the final patterns would not change. | ||
|
||
There is no way to restore the old behavior |
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.
Closing period?
Slightly change the algorithm to no longer clip the scaling factor, | ||
thus the patterns will continue to shrink at thin line widths. If the | ||
line width is smaller than the effective pixel size, this may result | ||
in dashed lines turning into solid gray lines. This also required |
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.
Why gray?
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 line was implicitly black in my head when I wrote this.
Small tweaks to API documentation on scaling dashes
The same test is failing on three environments: do we have a problem on 2.0.x with the tests? |
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 don't know if my review counts, but this looks good to me.
LGTM 👍
missing API docs from #8032