gh-137481: consider actual day name length

[snoyes]

Find the length of the longest day name in the current locale. Use that length, rather than a constant "9", to decide if the names should be abbreviated.

• Issue: Needless abbreviation of day names in certain locale in calendar #137481

[AA-Turner]

Please can you add a test (test_calendar.py) and a NEWS entry?

[bedevere-app]

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

[snoyes]

I have made the requested changes; please review again.

[bedevere-app]

Thanks for making the requested changes!

@AA-Turner: please review the changes made to this pull request.

[snoyes]

Please advise on the test - I can provide a few specific locales to test, such as Malay, Danish, and French, which illustrates the problem with the current code but might be too specific and so miss out on some other defect. Or I can loop through all available locales as in the latest commit, which is more comprehensive but increases the time to run this one test from a few hundredths of a second to over half a second. Is that a problem?

[serhiy-storchaka]

I prefer testing on a small set of example locales. I suspect that most locales are not affected by this change. Anyway, there is no portable way to get the list of all locales (locale.locale_alias is a bad substitute, and it contains many duplicates).

You can use the support.run_with_locale() or support.run_with_locales() decorators to run the test with different locales (the former runs with the first supported locale, the latter runs with all supported locales from the list).

[snoyes]

It's not most locales, but it's more than a dozen, including Chinese, Danish, French, Japanese, Korean, Malay, Norwegian, and Thai.

The same bug also produces the opposite error: if any weekday names are longer than 9 characters, then a calendar width of 9 simply truncates rather than using the proper abbreviation. That includes Bulgarian, Croatian, Finnish, German, Icelandic, Lithuanian, Polish, Portuguese, Russian, Sinhala, and Slovenian.

So the only locales not affected are those which happen to have exactly 9 characters in their longest weekday name.

I've changed the test case to use support.run_with_locale() and provided a short list, and added a second test case for the locales which ought to be properly abbreviated even if the supplied width exceeds 9 characters.

[serhiy-storchaka]

Thank you for your update.

Some lines are too long, please ensure that all lines do not exceed 79 columns, as required by PEP 8. If you give a name to lambda, using a local function is better.

Weekday names may be platform specific, so on particular platform the first found locale may have names not suitable for the test. I suggest to use run_with_locales() and skip the subtest for non-suitable locale. For example:

if max_length >= 9:
    self.skipTest('weekday names are too long')

[snoyes]

I have made the requested changes; please review again.

[bedevere-app]

Thanks for making the requested changes!

@AA-Turner: please review the changes made to this pull request.

[serhiy-storchaka]

LGTM. 👍

[serhiy-storchaka]

There are trailing whitespaces.

[snoyes]

Trailing whitespaces removed. I have made the requested changes; please review again.

[bedevere-app]

Thanks for making the requested changes!

@AA-Turner, @serhiy-storchaka: please review the changes made to this pull request.

[StanFromIreland]

This function is duplicated

[snoyes]

Do you mean the local get_weekday_names function? Or did you mean the test_locale_calendar_short_weekday_names vs test_locale_calendar_long_weekday_names ? The latter two do different things. The first tests that locales with short names are NOT abbreviated needlessly; the second tests that locales with long names ARE abbreviated rather than truncated.

[StanFromIreland]

I mean the helper, get_weekday_names

[snoyes]

Yes. Is that important in a test? Do you prefer that function be moved up to the global scope to avoid the duplication, or some other suggestion?

[kulikjak]

Hi, the new test case causes us build failures, because it depends on abbreviated names being three characters long, which is not always the case on Oracle Solaris.

I created #138156 which should make the test work with there cases as well.