X Tutup
Skip to content

DOC: Correct byweekday description in WeekdayLocator#31262

Open
kdpenner wants to merge 1 commit intomatplotlib:mainfrom
kdpenner:doc/update-byweekday-docstring
Open

DOC: Correct byweekday description in WeekdayLocator#31262
kdpenner wants to merge 1 commit intomatplotlib:mainfrom
kdpenner:doc/update-byweekday-docstring

Conversation

@kdpenner
Copy link
Contributor

@kdpenner kdpenner commented Mar 9, 2026

PR summary

The description of the default for byweekday in WeekdayLocator does not match the default in the signature.

PR checklist

  • [N/A] "closes #0000" is in the body of the PR description
  • [N/A] new and changed code is tested
  • [N/A] Plotting related features are demonstrated in an example
  • [N/A] New Features and API Changes are noted with a directive and release note
  • Documentation complies with general and docstring guidelines

Copy link
Contributor

@scottshambaugh scottshambaugh left a comment

Choose a reason for hiding this comment

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

This all checks out to me!

@kdpenner kdpenner force-pushed the doc/update-byweekday-docstring branch from 71790ea to 9d8fbbc Compare March 10, 2026 02:55
@kdpenner
Copy link
Contributor Author

just noticed that the tz types and description were incorrect as well

@QuLogic
Copy link
Member

QuLogic commented Mar 10, 2026

The tz docstring was not incorrect; that None is a sentinel, not an expected input: https://matplotlib.org/devdocs/devel/document.html#default-values

@kdpenner
Copy link
Contributor Author

I hate that. "the actual default" is None, and None is replaced by another value elsewhere in the stack. and None is a type accepted by the function, along with a string that parses to a timezone and a datetime.tzinfo type. however I can revert if necessary

@QuLogic
Copy link
Member

QuLogic commented Mar 10, 2026

It is impossible for the "actual" default to be rcParams['timezone'], so None is used as a sentinel, which is very common across Matplotlib and Python in general, as explained in the docs. Please revert that change.

@kdpenner
Copy link
Contributor Author

kdpenner commented Mar 10, 2026

I will revert but I stand by hating that as a user. there's a reason python has the Optional and | typing annotation syntax. also it is incorrect from a plain-English standpoint

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.

3 participants

X Tutup