Skip to content

REORG: JoinStyle and CapStyle classes #18544

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

Merged
merged 4 commits into from
Feb 1, 2021

Conversation

brunobeltran
Copy link
Contributor

@brunobeltran brunobeltran commented Sep 22, 2020

Centralize docs and validation for JoinStyle and CapStyle in one place.

On top of #18817 #19063.

PR Summary

First GSOD PR. Follow suggestion of @anntzer in MEP 30 to simply start making the simple string enum cases before moving onto more complicated things like LineStyle.

The primary goal of this PR is to create a centralized place to document what is a JoinStyle anytime that parameter is used:

class Collection(...):
    ...
    def __init__(...):
    """
    ...
        joinstyle : `.JoinStyle`-like, default: :rc:`patch.joinstyle`
            Style to use for joining lines for all paths in the collection.
    """"

This PR accomplishes this by adding (totally transparent) JoinStyle and CapStyle classes, which act as both a centralized location for documentation of these "features" of our library, and as validators (and, hopefully, in the future, serializers for rc).

The user can still use the standard string input style, i.e. this still works:

plt.plot(x, y, joinstyle='miter')

Up until this point, I am basically just implementing the GSOD in the way which was agreed upon in the last dev call about this. But there are some additional considerations that come up once you start to implement this.

As soon as any end-user-facing function takes a joinstyle argument, we have two options. Either we can immediately convert:

def __init__(..., joinstyle=JoinStyle.round):
... # ValueError if invalid `joinstyle`
    self._joinstyle = JoinStyle(joinstyle)

or we can validate the input but store the string:

def __init__(..., joinstyle='round'):
... # ValueError if invalid `joinstyle`
    self._joinstyle = joinstyle if JoinStyle(joinstyle)

The former has the upside of being in line with what we already do elsewhere in the library (e.g. MarkerStyle), while the latter has the upside of breaking less API surface naively (EDIT: although, as I point out to Tim in the comments below, this can be very strongly mitigated with enough care in implementing dunder methods for backcompat).

This PR implements the approach where we immediately convert (the former of the two above).

PR Checklist

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (run flake8 on changed files to check).
  • New features are documented, with examples if plot related.
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • Conforms to Matplotlib style conventions (install flake8-docstrings and pydocstyle<4 and run flake8 --docstring-convention=all).
  • [N/A] New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • [N/A] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).

(Note: Tom explicitly does not want this new API surface advertised (yet)).

@brunobeltran brunobeltran marked this pull request as draft September 22, 2020 19:00
@brunobeltran brunobeltran force-pushed the join_cap_styles branch 5 times, most recently from d5ec47d to dd34f72 Compare September 30, 2020 21:50
@brunobeltran brunobeltran marked this pull request as ready for review September 30, 2020 21:51
@brunobeltran
Copy link
Contributor Author

Alright @timhoffm, these docs now look like what I would expect when I build them locally. Feel free to offer any feedback, but some points I'm most interested in your opinion are:

  1. How to deal with "constructible-as". I made sure that everywhere which now takes a [Cap/Join]Style still takes the old string inputs by simply calling the new constructors immediately. This means that functions with those arguments will have Parameters entries which look like:
joinstyle : `JoinStyle` or {'bevel', 'miter', 'round'}, default: :rc:`patches.joinstyle`

or similar.

Frankly, this is totally fine, but for other types, like LineStyle, this will quickly get out of hand. I propose the following standard:
we list out both the "_types" type and the alternative options (e.g. LineWidth or float) unless that would make us overflow the line. This has the upside of keeping things as easy to discover as possible without sacrificing readability (and I would argue that if you don't know what all the options for e.g. LineStyle are, I do want to force you to click that link or type "help()" once more.

Another option is to do something similar to what numpy does, and have some syntax for "this parameter can be any type that can be passed directly to the constructor of X". "X-like", "X-able", "X-constructible" or something similar. This would make the above docstring look like

joinstyle : `JoinStyle`-like, default: :rc:`patches.joinstyle`

which is of course much shorter.

  1. Obviously I'm a fan of your suggestion to call this module _types.py, but any other naming thoughts are more than welcome!

  2. and finally, I guess do we want this to get "what's new"? I guess if we want this to be solely a documentation/internal validator mechanism for now, I could just make sure the entry emphasizes that this private module is open for use, but that it's name/etc. are liable to change in a future version?

Comment on lines 9 to 41
def _deprecate_case_insensitive_join_cap(s):
s_low = s.lower()
if s != s_low:
if s_low in ['miter', 'round', 'bevel']:
cbook.warn_deprecated(
"3.3", message="Case-insensitive capstyles are deprecated "
"since %(since)s and support for them will be removed "
"%(removal)s; please pass them in lowercase.")
elif s_low in ['butt', 'round', 'projecting']:
cbook.warn_deprecated(
"3.3", message="Case-insensitive joinstyles are deprecated "
"since %(since)s and support for them will be removed "
"%(removal)s; please pass them in lowercase.")
# Else, error out at the check_in_list stage.
return s_low
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deprecation machinery copied as-is out of rcsetup.py.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems arbitrary, and should definitely be a class method if you really want to keep it in. Why would we care if join styles are capitalized or not? Do we really think "round" and "Round" will ever mean something different?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is existing behavior. I was not around for the discussion about whether or not to allow caps. I agree that for a user-facing thing, allowing case-insensitivity is probably fine, but this should probably not be hashed out in this PR.

Comment on lines 87 to 111
def demo():
import numpy as np
import matplotlib.pyplot as plt

def plot_angle(ax, x, y, angle, style):
phi = np.radians(angle)
xx = [x + .5, x, x + .5*np.cos(phi)]
yy = [y, y, y + .5*np.sin(phi)]
ax.plot(xx, yy, lw=12, color='tab:blue', solid_joinstyle=style)
ax.plot(xx, yy, lw=1, color='black')
ax.plot(xx[1], yy[1], 'o', color='tab:red', markersize=3)

fig, ax = plt.subplots(figsize=(8, 6))
ax.set_title('Join style')
for x, style in enumerate(['miter', 'round', 'bevel']):
ax.text(x, 5, style)
for y, angle in enumerate([20, 45, 60, 90, 120]):
plot_angle(ax, x, y, angle, style)
if x == 0:
ax.text(-1.3, y, f'{angle} degrees')
ax.set_xlim(-1.5, 2.75)
ax.set_ylim(-.5, 5.5)
ax.set_axis_off()

plt.show()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Demo copied as-is from gallery/lines_bars_and_markers/joinstyle.html

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure about the demo. On the one hand, it may be nice to be able to display the options. On the other hand, I don't like the code duplication with the example. That will get out of sync sooner or later. Also, if the user is calling demo() while working on a figure, plt.show() will display that figure and thus mess up the user's work.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, sorry, I meant to remove that example, it's gone now.

Obviously I don't think this type of documentation should replace examples, but in this case (where the example was literally just a demo of the definitions of each available style), I think that particular documentation should live with the class, where it won't go out of sync.

Also, if the user is calling demo() while working on a figure, plt.show() will display that figure and thus mess up the user's work.

This is a good point, I changed the demo to only "show" its own figure.

Comment on lines 150 to 199
import matplotlib.pyplot as plt

fig, ax = plt.subplots(figsize=(8, 2))
ax.set_title('Cap style')

for x, style in enumerate(['butt', 'round', 'projecting']):
ax.text(x+0.25, 1, style, ha='center')
xx = [x, x+0.5]
yy = [0, 0]
ax.plot(xx, yy, lw=12, color='tab:blue', solid_capstyle=style)
ax.plot(xx, yy, lw=1, color='black')
ax.plot(xx, yy, 'o', color='tab:red', markersize=3)
ax.text(2.25, 0.7, '(default)', ha='center')

ax.set_ylim(-.5, 1.5)
ax.set_axis_off()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Demo copied as-is from gallery/lines_bars_and_markers/joinstyle.html

from matplotlib.texmanager import TexManager
from matplotlib.transforms import Affine2D
from matplotlib.backends.backend_mixed import MixedModeRenderer
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

alphabetization

@@ -110,14 +111,10 @@ def __init__(self,
where *onoffseq* is an even length tuple of on and off ink lengths
in points. For examples, see
:doc:`/gallery/lines_bars_and_markers/linestyles`.
capstyle : str, default: :rc:`patch.capstyle`
capstyle : `.CapStyle`-like, default: :rc:`patch.capstyle`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timhoffm This would be an example where including the individual possibilities would push us over a single line.

Comment on lines 25 to 29
from matplotlib import _api, animation, cbook
from matplotlib import animation, cbook
from matplotlib.cbook import ls_mapper
from matplotlib.fontconfig_pattern import parse_fontconfig_pattern
from matplotlib.colors import is_color_like
from matplotlib.fontconfig_pattern import parse_fontconfig_pattern
from matplotlib._types import JoinStyle, CapStyle
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tl;dr: alphabetized, added _types, removed _api

@brunobeltran
Copy link
Contributor Author

CI failing on backends that I don't have installed locally (surprise). I'll do a better job of cleaning that up ASAP.

But this also goes to show that with this specific proposal, this is a major API change for people who write their own backends. I'll bring this up on the next call.

@timhoffm
Copy link
Member

timhoffm commented Sep 30, 2020

tl/dr;

Not yet decided. I have to think about this more. Everything below is just unstructured preliminary thoughts.

Private/public

I'm hesitant to make this public right now. Using these types on the front-end is a lot bulkier:

from matplotlib._types import JoinStyle
plt.plot(x, y, joinstyle=JoinStyle.miter)

instead of

plt.plot(x, y, joinstyle='miter')

I can't bring myself to recommending or even only showing the above.

Type documentation

In turn, if JoinStyle is not public, we should not announce it as a supported parameter type.

For completeness: If the types were public:

  1. We should try not to overflow the line.
  2. Explicit is better:
    If it matches on the line:
    joinstyle : `JoinStyle` or {'bevel', 'miter', 'round'}
    
    otherwise (we already use "array-like"):
    joinstyle : `JoinStyle`-like
    

Alternatives

If you are motivated to dive into sphinx roles:

joinstyle : :mpltype:`JoinStyle`

With a suitable rendered representation. The downside is that inline help does not have the explicit values.

@story645
Copy link
Member

story645 commented Oct 1, 2020

There is the "provisional" label for making it semi-public if you want a middle ground.

@brunobeltran
Copy link
Contributor Author

brunobeltran commented Oct 1, 2020

I'm hesitant to make this public right now. Using these types on the front-end is a lot bulkier:

The way the type is constructed is completely transparent to the user. I would argue that even if this was fully implemented, we would still use the "string" versions in all of our examples, and encourage users to do so (due to the added conciseness), except in examples where there's an obvious readability benefit to constructing the type bit-by-bit (like making a complicated custom MarkerStyle).

in other words, this still works:

plt.plot(x, y, joinstyle='miter')

IMO the real issue isn't whether or not we promote this to our end users (since they are really not affected except for they get the extra option of passing in these guys as the explicit object if they really want), but whether we internally store the string (e.g. in GraphicsContextBase) or store the new Enum object (in which case third-party backends would have to add .name anywhere they use the JoinStyle as a dictionary key)...[see footnote]

I can't bring myself to recommending or even only showing the above.
The benefits are clearly not there for a simple case like JoinStyle, but the goal is to make objects like LineStyle more useable in addition to making their advanced features more discoverable (similarly to how MarkerStyle gets its own class, but nobody complains because you can still just pass 'o' or similar).

If you are motivated to dive into sphinx roles:

joinstyle : :mpltype:`JoinStyle`

This was an alternate proposal that I fully wrote up at one point. In the last dev call where this was discussed, it was decided that making them full classes makes more sense, as this gives us a place to put validators/serializers (since I'll be writing the validators anyway given that it's zero extra work once I commit to fully documenting these things).

@timhoffm if you think it would be worthwhile I'm happy to have this role implemented for Monday, since I've done the sphinx role dance before, if you think having these two options would aid discussion?

....

[footnote]
...although as a side note, we could even do an arbitrarily good job handling backcompat, by overwriting __hash__ and changing the default Enum dunder functions to be more friendly, e.g.:

def __eq__(self, other):
    try:
        other = self.__class__(other)
    except ValueError:
        return False
    return self == other

and so forth. I could make a "Wasn'tAClassBeforeBase" class to share all of these "fixes"....

@jklymak
Copy link
Member

jklymak commented Oct 1, 2020

I think this is useful, so long as the old behaviour is maintained of allowing a string. The point of making these public is that they will show up as their own thing in the docs.

OTOH, I don't know about calling this module "types". That is a pretty generic and loaded term in computing. style_classes? Dunno, but not types!

@timhoffm
Copy link
Member

timhoffm commented Oct 1, 2020

@jklymak We already have a style module. That name is taken and would be too confusing to be used with a different context.

Requirements

(sort of, style classes are already a solution, but hey)

  • we want the style classes for internal stuff (validating, serializing, maybe as internal datatype)
  • we want a dedicated place to put docs for these and have links from corresponding parameters.
  • user should still only use the simple types (e.g. strings)

Proposed solution

  1. Let's keep the style classes private. If users shouldn't use them, they don't need to know that there is a formal JoinStyle class. This also alleviates the module naming discussion.
  2. Style classes collect all the desired functionality, including documentation.
  3. We don't put the classes in our public docs using the standard sphinx API generation tools (these classes are private, see 1.). Instead, we need another mechanism to extract the documentation and put it on some pages, possibly a custom sphinx directive. To be decided whether these docs are the class docstring or some other string attached to the class.
  4. We also need a mechanism to link types, possibly a sphinx role. Maybe (up for discussion):
    :mpltype:`joinstyle` with an option for :mpltype:`joinstyle {'miter', 'round', 'bevel'}` if we want to state explicit values for plaintext docs.

Defining own sphinx roles and directives is a bit more heavy-weight, but it allows us to use internal code structures and keep them completely hidden from the user.

@jklymak
Copy link
Member

jklymak commented Oct 1, 2020

But a counterpoint is that if we have public classes they could take kwargs on init. I imagine some of those have parameters that could be adjusted. We do exactly this for markers.

@timhoffm
Copy link
Member

timhoffm commented Oct 1, 2020

(Some of) the classes can still be made public if we feel that is beneficial.

@jklymak
Copy link
Member

jklymak commented Oct 1, 2020

I'd agree with you, if you hadn't just proposed a new sphinx directive and a new sphinx role. The more of these we have, the more fragile our development process is.

@brunobeltran
Copy link
Contributor Author

brunobeltran commented Oct 2, 2020

* user should still only use the simple types (e.g. strings)

@timhoffm, this phrasing is surprising to me, based on the concerns you voiced above, I would have expected you to say:

*user should still be able to use the simple types (e.g. strings)

I definitely agree with the latter, and I think no proposal can move forward that doesn't allow the user to still use the string syntax, but I don't see the danger/negative to allowing the user to use the new syntax, even if in this case it's strictly more verbose....

In particular, this would solve the tab-complete dilemma that I've seen you be actively involved in before, right? I mean if the user does take the time to from _types import *, then they can do ..., lw=5, joinstyle=JoinStyle.[TAB].

And as we all know, documentation-by-available-tab-completion-options is a real thing, and iirc Jedi is not stoked about getting this to work in general for string parameters (even with options listed in docstring?)

@brunobeltran
Copy link
Contributor Author

That is a pretty generic and loaded term in computing. style_classes? Dunno, but not types!

I had originally called the module _styles, but was 'suaded by Tim's argument about it being too close to "style". I would argue that the whole module is private for now so we are free to hash out synonyms to "styles" or "types" for some time before we commit.

@timhoffm
Copy link
Member

timhoffm commented Oct 2, 2020

I don't see the danger/negative to allowing the user to use the new syntax, even if in this case it's strictly more verbose....

On a technical level that should work. I'm more concerned on "there should only be one way" for the parameters. Are the new classes hidden / mentioned but not emphasized / an equal alternative? For now, I tend towards not mentioning them to the user at all to not increase the perceived API complexity.

You have a point with tab-completion. Unfortunately, jedi will indeed not support string completion from docstrings. However, there's a chance to get string completion from typing at some point davidhalter/jedi#1355.

@timhoffm
Copy link
Member

timhoffm commented Oct 2, 2020

I'd agree with you, if you hadn't just proposed a new sphinx directive and a new sphinx role. The more of these we have, the more fragile our development process is.

I disagree on fragile. If they are well written and documented, additional roles do not complicate development. In contrast, they can help to generate better, more consistent documentation. E.g. you could auto-generate the possible string values in the HTML docs. Or, if you want explicit values for text docstrings like :mpltype:`joinstyle {'miter', 'round', 'bevel'}` , the role could validate that the stated texts are correct.

@brunobeltran
Copy link
Contributor Author

brunobeltran commented Oct 2, 2020

On a technical level that should work. I'm more concerned on "there should only be one way" for the parameters.

Hopefully not getting too philosophical here, but while I am always behind have "One Way™" to do things, I don't think allowing the user more options for the type to be passed as a parameter counts as having "more ways". To the contrary, I think it's pretty standard fare---especially in the scientific Python ecosystem---to expect that the input you pass to a function might be converted internally to a more precise representation....I mean even setting numpy aside look at dict_keys or other things even in core Python. From my perspective the etiquette for doing something like this as a library writer seems to be that it's "Okay™" as long as the user doesn't have to know about the internal representation to get the job done (which is still true here).

so I don't know if any users would even blink about it any more than they would be confused whether they are allowed to pass a list or a tuple to certain inputs....

...okay fine so users do get confused about that but I would argue that's just how things are done in Python.

Are the new classes hidden / mentioned but not emphasized / an equal alternative? For now, I tend towards not mentioning them to the user at all to not increase the perceived API complexity.

I don't see any reason reason to mention them, but presumably they will be discovered naturally by users that are reading the documentation deeply anyway, so I don't see any reason to "hide" them either. As long as both the string and enum versions still quack, I don't see users getting upset.

You have a point with tab-completion. Unfortunately, jedi will indeed not support string completion from docstrings. However, there's a chance to get string completion from typing at some point davidhalter/jedi#1355.
How this interacts with Typing is an interesting, but I guess very future discussion, (but IMO having it be a "real" class would greatly simplify any implementation of "types" in the future, even thought we keep the "string" versions forever).

@jklymak
Copy link
Member

jklymak commented Oct 2, 2020

Again, we do this for markers; either take a string or a class, and I don't think its a problem at all, and leaves the door open for users to write new styles. We could do the same for line styles, and that would get us part way towards allowing folks to write their own brush stroke patterns for lines, or complicated dot-dash patterns without ugly strings to parse.

@brunobeltran brunobeltran force-pushed the join_cap_styles branch 2 times, most recently from 63c5c77 to e7353a1 Compare October 3, 2020 22:51
@timhoffm
Copy link
Member

timhoffm commented Oct 4, 2020

One of the questions influencing the solution space is how we want the docs in rendered and source from to describe these types.

Possible rendered docs:

1) joinstyle : {'miter', 'round', 'bevel'}
2) joinstyle : JoinStyle
3) joinstyle : JoinStyle-like
5) joinstyle : JoinStyle {'miter', 'round', 'bevel'}
6) joinstyle : {'miter', 'round', 'bevel'} or JoinStyle

Possible raw docs:

1) joinstyle : {'miter', 'round', 'bevel'}

2) joinstyle : `JoinStyle`

3) joinstyle : `JoinStyle`-like

4) joinstyle : :mpltype:`{'miter', 'round', 'bevel'}`

5) joinstyle : :mpltype:`JoinStyle`

5b) joinstyle : :mpltype:`JoinStyle {'miter', 'round', 'bevel'}`

6) joinstyle : {'miter', 'round', 'bevel'} or `JoinStyle`
  1. is the current way and not what we want as it has no link.
    2)+3) Could be used with type classes and standard numpydoc/sphinx documentation machinery.
    4)+5) Need a custom sphinx role. Due to each of the raw docs could generate either rendered version.
    Raw 4) needs a special internal lookup, that associates "{'miter', 'round', 'bevel'}" with JoinStyle.
  2. Is straight forward, but has the drawback that strings cannot be checked/auto-generated.

I'm inclined towards 4). Because it states the string values most users would use both in raw and HTML, but still links to the style docs.

@jklymak
Copy link
Member

jklymak commented Oct 4, 2020

How about:

joinstyle : {'miter', 'round', 'bevel'} or `JoinStyle`

@timhoffm
Copy link
Member

timhoffm commented Oct 4, 2020

Yes added to the list and scheduled for discussion in the dev call.

@brunobeltran brunobeltran force-pushed the join_cap_styles branch 2 times, most recently from 288fe5d to 858ee17 Compare February 1, 2021 16:42
@timhoffm timhoffm merged commit 651c293 into matplotlib:master Feb 1, 2021
@timhoffm timhoffm added this to the v3.4.0 milestone Feb 1, 2021
stonebig added a commit to stonebig/holoviews that referenced this pull request Apr 3, 2021
philippjfr pushed a commit to holoviz/holoviews that referenced this pull request Apr 8, 2021
* Matplotlib-3.4.0 compatibility 

following this API change matplotlib/matplotlib#18544

* Update util.py

* Update util.py
StefRe added a commit to StefRe/matplotlib that referenced this pull request Dec 27, 2021
This was certainly overlooked in matplotlib#18544, otherwise it would make
no sense to derive CapStyle from _AutoStringNameEnum.
StefRe added a commit to StefRe/matplotlib that referenced this pull request Dec 27, 2021
This was certainly overlooked in matplotlib#18544, otherwise it would make
no sense to derive CapStyle from _AutoStringNameEnum.
scottshambaugh pushed a commit to scottshambaugh/matplotlib that referenced this pull request Jan 6, 2022
This was certainly overlooked in matplotlib#18544, otherwise it would make
no sense to derive CapStyle from _AutoStringNameEnum.
QuLogic added a commit to QuLogic/matplotlib that referenced this pull request May 6, 2023
When these Enum classes were added in matplotlib#18544, they were supposed to be
for documentation only. To that end, matplotlib#22055 was a followup that ensured
that only the strings were exposed from the getter side.

However, when user-supplied cap/join style were added in matplotlib#20914, they
were only for the Enum type instead of the string, so correctly allow
strings here as well.

Also, specifically type hint the return values as literals, as was done
in matplotlib#25719.
QuLogic added a commit to QuLogic/matplotlib that referenced this pull request May 6, 2023
When these Enum classes were added in matplotlib#18544, they were supposed to be
for documentation only. To that end, matplotlib#22055 was a followup that ensured
that only the strings were exposed from the getter side.

However, when user-supplied cap/join style were added in matplotlib#20914, they
were only for the Enum type instead of the string, so correctly allow
strings here as well.

Also, specifically type hint the return values as literals, as was done
in matplotlib#25719.
@QuLogic QuLogic mentioned this pull request May 6, 2023
1 task
QuLogic added a commit to QuLogic/matplotlib that referenced this pull request May 31, 2023
When these Enum classes were added in matplotlib#18544, they were supposed to be
for documentation only. To that end, matplotlib#22055 was a followup that ensured
that only the strings were exposed from the getter side.

However, when user-supplied cap/join style were added in matplotlib#20914, they
were only for the Enum type instead of the string, so correctly allow
strings here as well.

Also, specifically type hint the return values as literals, as was done
in matplotlib#25719.
devRD pushed a commit to devRD/matplotlib that referenced this pull request Jun 5, 2023
When these Enum classes were added in matplotlib#18544, they were supposed to be
for documentation only. To that end, matplotlib#22055 was a followup that ensured
that only the strings were exposed from the getter side.

However, when user-supplied cap/join style were added in matplotlib#20914, they
were only for the Enum type instead of the string, so correctly allow
strings here as well.

Also, specifically type hint the return values as literals, as was done
in matplotlib#25719.
melissawm pushed a commit to melissawm/matplotlib that referenced this pull request Jun 15, 2023
When these Enum classes were added in matplotlib#18544, they were supposed to be
for documentation only. To that end, matplotlib#22055 was a followup that ensured
that only the strings were exposed from the getter side.

However, when user-supplied cap/join style were added in matplotlib#20914, they
were only for the Enum type instead of the string, so correctly allow
strings here as well.

Also, specifically type hint the return values as literals, as was done
in matplotlib#25719.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants