Skip to content
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: Best practice when to use ax.set_thing(foo) vs ax.set(thing=foo) #28693

Open
timhoffm opened this issue Aug 9, 2024 · 3 comments
Open

Comments

@timhoffm
Copy link
Member

timhoffm commented Aug 9, 2024

Documentation Link

No response

Problem

Inspired through the discussion at #28677 (comment): Through #20569 we have made Artist.set() more useable. In particular, we have started to Axes.set() in some places in the documentation.

We currently do not have a best practice when to ax.set_thing(foo) vs ax.set(thing=foo).

Suggested improvement

We should have a best practive when to use what that we document, teach and use in our docs.

There was a loose discussion at https://github.com/matplotlib/ProjectManagement/blob/b9b0a56cb7b329c3c2adfac77a84975f70868e24/meeting_notes/2021/q2.md#do-we-want-to-soup-up-and-advertise-objset-more, but no definitive solution.

General observations:

  • Both APIs are intended to co-exist. We don't plan to get rid of one or the other.
  • Advantages of set_thing():
    • more specific
    • easier to find the docs (for set(thing=...) one has to go to set() and follow the link for the specific keyword).
    • more functionality through kwargs available
  • Advantages of set(thing=...):
    • shorter
  • Neutral
    • tab completion should work for both cases (ax.set_<TAB> and ax.set(<TAB>)
    • API is fully documented for both cases (no more set(**kwargs) signatures)
    • Historically set_thing was mostly used and advertized

Please add if you have additional points.

I'll stop here without suggesting a solution to keep the discussion unbiased. Suggestions for solution should be posted as separte comments.

@timhoffm
Copy link
Member Author

timhoffm commented Aug 9, 2024

Proposal:

Use ax.set_thing() as a baseline. These are well-known and less experienced people who only learn one interface can discover it easily and can potentially do everything with it. Consider and teach ax.set() as a shortcut that can be used to collect multiple set_ * into a single command. IMHO this is enough level of detail for the end user.

Our docs, have higher bar on being understandable and instructive. I therefore would be a bit more specific. As rules of thumb for the matplotlib docs:

  • Use set() if you want to set multiple similar properties (and don't need additional parameters), e.g.
    • set(xlim=..., ylim=...)
    • set(xlim=..., xlabel=...)
    • set(xlabel=..., ylabel=..., title=...)
    • set >= 3 properties
  • Do not use set() for
    • single properties, e.g. set(title=...)
    • two properties that are very different, e.g. set(title=..., xlim=...) - saving one line is not worth the reduced clarity of doing two very different things in one call.

@rcomer
Copy link
Member

rcomer commented Aug 9, 2024

I agree that set_thing is canonical and set is a shortcut. I think whether the shortcut is appropriate for a particular use-case is up to the user to decide.

For the docs/examples, I think @timhoffm and @story645 both make good arguments in opposing directions and I do not know how to choose between them! So I would vote for not codifying this but instead leave it up to individual authors and their reviewers. This also fits my general preference for not making the contributors’ guide any larger than it needs to be.

@story645
Copy link
Member

story645 commented Aug 9, 2024

This also fits my general preference for not making the contributors’ guide any larger than it needs to be

My preference for consistency in the docs means I'm okay w/ the decision not going in my favor, and i think adding this to an entry in the style guide would be OK? I see that as sorta the doc equivalent to the PR guidelines, more for maintainers than contributors, but also means then we don't have long discussions about it on every PR.

I also agree w/ you very much that really it's so very case dependent. I think Tim's first bullet is a good compromise:

set >= 3 properties

Anything more than about 2 properties and the set_* boilerplate starts getting overwhelming and distracts from the purpose of the example, two very different properties or just one property and use of set might be what's distracting.

two properties that are very different, e.g. set(title=..., xlim=...) - saving one line is not worth the reduced clarity of doing two very different things in one call.

I love highlighting that set works for all the things & is the wonderful all the things function folks want.

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

No branches or pull requests

3 participants