Skip to content

gh-131146: Fix month names in a genitive case in calendar module #131147

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

plashchynski
Copy link

@plashchynski plashchynski commented Mar 12, 2025

The calendar module displays month names in some locales using the genitive case. This is grammatically incorrect, as the nominative case should be used when the month is named by itself. To address this issue, this change introduces new lists alt_month_name and alt_month_abbr that contain month names in the nominative case. The module now uses %OB format specifier to get month names in the nominative case where available.

The calendar module displays month names in some locales using the genitive case.
This is grammatically incorrect, as the nominative case should be used when the month
is named by itself. To address this issue, this change introduces new lists
`alt_month_name` and `alt_month_abbr` that contain month names in the nominative case.
The module now uses `%OB` format specifier to get month names in the nominative case
where available.
@ghost
Copy link

ghost commented Mar 12, 2025

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

bedevere-app bot commented Mar 12, 2025

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Copy link
Member

@tomasr8 tomasr8 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! This will also need a NEWS entry and tests (check Lib/test/test_calendar.py)

* Limit line length to 79 characters according to PEP8
* Suppress current unit links according to the Documentation Style Guide
@hugovk hugovk changed the title gh-131146: Fix month names in a genitive case in calendar module. gh-131146: Fix month names in a genitive case in calendar module Mar 24, 2025
Comment on lines +496 to +498
A sequence that represents the months of the year in the current locale.
This follows normal convention of January being month number 1, so it has
a length of 13 and ``month_name[0]`` is the empty string.
Copy link
Member

Choose a reason for hiding this comment

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

This is just a formatting change and should not be included in this PR.

Copy link
Author

Choose a reason for hiding this comment

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

It's to limit the line length to 80 characters in accordance with the reStructuredText markup guideline

Copy link
Member

Choose a reason for hiding this comment

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

Again, you did not modify this section, please revert it. Existing violations do not have to be fixed, I assure you there are hundreds in the file.

Comment on lines +516 to +517
locale. This follows normal convention of January being month number 1, so
it has a length of 13 and ``month_abbr[0]`` is the empty string.
Copy link
Member

Choose a reason for hiding this comment

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

Again unrelated formatting changes

Copy link
Author

Choose a reason for hiding this comment

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

It's to limit the line length to 80 characters in accordance with the reStructuredText markup guideline.

Copy link
Member

Choose a reason for hiding this comment

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

You did not modify this section, it is unrelated to this pr, please revert it.

Comment on lines +146 to +152
try:
standalone_month_name = _localized_month('%OB')
standalone_month_abbr = _localized_month('%Ob')
except ValueError:
# The platform does not support the %OB and %Ob specifiers.
standalone_month_name = month_name
standalone_month_abbr = month_abbr
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
try:
standalone_month_name = _localized_month('%OB')
standalone_month_abbr = _localized_month('%Ob')
except ValueError:
# The platform does not support the %OB and %Ob specifiers.
standalone_month_name = month_name
standalone_month_abbr = month_abbr
try:
standalone_month_name = _localized_month('%OB')
standalone_month_abbr = _localized_month('%Ob')
except ValueError:
standalone_month_name = month_name
standalone_month_abbr = month_abbr

The previous comment covers this IMO

Comment on lines +1 to +3
Add :data:`calendar.standalone_month_name` and :data:`calendar.standalone_month_abbr` to
provide month names and abbreviations in a grammatical form used when a month name stands
by itself if the locale provides one.
Copy link
Member

Choose a reason for hiding this comment

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

Both the NEWS entries are for the same issue, they could just be merged. "Fix ... by adding :data:calendar.standalone_month_name and :data:calendar.standalone_month_abbr. These provide ... "

@encukou encukou added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 29, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @encukou for commit 36bb456 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131147%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Apr 29, 2025
@encukou
Copy link
Member

encukou commented Apr 29, 2025

Thank you for the PR, @plashchynski
If you don't beat me to it, I plan to fix the formatting/comments/entries tomorrow so that this gets into 3.14.

@StanFromIreland
Copy link
Member

This also needs a proper test (my comment was incorrectly closed) using support's @run_with_locale, grep the existing tests for usage examples, there are quite a few.

Comment on lines +537 to +540
>>> import calendar
>>> list(calendar.standalone_month_name)
['', 'January', 'February', 'March', 'April', 'May', 'June', 'July', 'August', 'September', 'October', 'November', 'December']

Copy link
Member

@StanFromIreland StanFromIreland Apr 29, 2025

Choose a reason for hiding this comment

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

This example is pointless in the English locale, and it links to month_name with the exact same example.

Copy link
Author

Choose a reason for hiding this comment

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

I agree, it's not relevant for English. But which locale should I use here?

Copy link
Member

Choose a reason for hiding this comment

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

We do not have to provide an example.

Comment on lines +551 to +554
>>> import calendar
>>> list(calendar.standalone_month_abbr)
['', 'Jan', 'Feb', 'Mar', 'Apr', 'May', 'Jun', 'Jul', 'Aug', 'Sep', 'Oct', 'Nov', 'Dec']

Copy link
Member

Choose a reason for hiding this comment

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

Ditto

Comment on lines 532 to 534
A sequence that represents the months of the year in the current locale
in the grammatical form used when a month name stands by itself if the locale
provides one. If the locale does not supply an alternative form, it is equal to
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
A sequence that represents the months of the year in the current locale
in the grammatical form used when a month name stands by itself if the locale
provides one. If the locale does not supply an alternative form, it is equal to
A sequence that represents the months of the year in the current locale
in the standalone form if the locale provides one. Else it is equivalent to

Much simpler format, as docs should be e.g.

Comment on lines 532 to 534
A sequence that represents the months of the year in the current locale
in the grammatical form used when a month name stands by itself if the locale
provides one. If the locale does not supply an alternative form, it is equal to
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
A sequence that represents the months of the year in the current locale
in the grammatical form used when a month name stands by itself if the locale
provides one. If the locale does not supply an alternative form, it is equal to
A sequence that represents the months of the year in the current locale
in the standalone form if the locale provides one. Else it is equivalent to

Much simpler format, as docs should be e.g.

@StanFromIreland
Copy link
Member

I can finish this off if you are unable to, it should still be backported to 3.14 since it is a bug fix, no?

@StanFromIreland

This comment was marked as resolved.

@plashchynski
Copy link
Author

This also needs a proper test (my comment was incorrectly closed) using support's @run_with_locale, grep the existing tests for usage examples, there are quite a few.

The problem is specific only to certain locales. To properly test it, we need to have these locales installed. A test with @run_with_locale will fail only on systems where that locales are installed. So for most systems that test will be green without proposed changes. I think it's not worth to add tests that can't fail on most of the systems.

@StanFromIreland
Copy link
Member

So for most systems that test will be green without proposed changes.

It will not pass, it will be skipped.

I think it's not worth to add tests that can't fail on most of the systems.

It is! Most systems have locale like pl_PL installed anyway, looking at the buildbots the majority do.

@plashchynski
Copy link
Author

plashchynski commented May 11, 2025

I think it's not worth to add tests that can't fail on most of the systems.

It is! Most systems have locale like pl_PL installed anyway, looking at the buildbots the majority do.

Even if the system has pl_PL installed, it's possible that it doesn't support %OB %Ob extensions. And we don't know for sure weather it supported or not.

Given this, how could we distinguish the wrong behaviour from the fallback route?
I mean, if we test for the standalone form, it's possible that the regular form is returned because of the fallback.
Hope it makes sense.

@encukou
Copy link
Member

encukou commented May 19, 2025

One trick would be to check for some common platform that does support %OB, for example:

if platform.libc_ver()[0] == 'glibc':
    "Polish month 2 MUST be 'luty'"
else:
    "Polish month 2 could be either 'luty' or 'lut'"
@serhiy-storchaka
Copy link
Member

The test should check for example that calendar.standalone_month_name[5] is contained in the calendar output for May. It can also skip if calendar.standalone_month_name[5] == calendar.month_name[5].

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