bpo-40275: Remove duplicate import of locale module by shihai1991 · Pull Request #19761 · python/cpython · GitHub
Skip to content

bpo-40275: Remove duplicate import of locale module - #19761

Merged
vstinner merged 6 commits into
python:masterfrom
shihai1991:bpo_40275_locale
May 4, 2020
Merged

bpo-40275: Remove duplicate import of locale module#19761
vstinner merged 6 commits into
python:masterfrom
shihai1991:bpo_40275_locale

Conversation

@shihai1991

@shihai1991 shihai1991 commented Apr 28, 2020 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@shihai1991 shihai1991 changed the title WIP: Remove duplicate import of locale module bpo-40275: Remove duplicate import of locale module Apr 29, 2020
vstinner
vstinner requested changes Apr 29, 2020
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR goes somehow against https://bugs.python.org/issue40275 goal. I would prefer to move the top-level "import locale" into skip_if_buggy_ucrt_strfptime().

@bedevere-bot

bedevere-bot commented Apr 29, 2020

Copy link
Copy Markdown

@shihai1991

shihai1991 commented Apr 29, 2020

Copy link
Copy Markdown
Member Author

This PR goes somehow against https://bugs.python.org/issue40275 goal. I would prefer to move the top-level "import locale" into skip_if_buggy_ucrt_strfptime().

make sense, I updated this PR.

@shihai1991

shihai1991 commented Apr 29, 2020

Copy link
Copy Markdown
Member Author

Oh, the gate of macos failed randomly (no generated log file)

@serhiy-storchaka

serhiy-storchaka commented Apr 29, 2020 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

It does not help. locale is imported by gettext which is imported by argparse which is imported by unittest.

For the purpose of bpo-40275 the changes should reduce the list of imported modules.

@vstinner

vstinner commented Apr 29, 2020

Copy link
Copy Markdown
Member

It does not help. locale is imported by gettext which is imported by argparse which is imported by unittest.

Maybe we could attempt to make locale import lazy in gettext, or make gettext import lazy in argparse.

Or just give up because locale is less heavy than asyncio for example :-)

@shihai1991

shihai1991 commented May 1, 2020

Copy link
Copy Markdown
Member Author

It does not help. locale is imported by gettext which is imported by argparse which is imported by unittest.

Maybe we could attempt to make locale import lazy in gettext, or make gettext import lazy in argparse.

Or just give up because locale is less heavy than asyncio for example :-)

I try to make local import lazy in gettext. I don't make gettext import lazy in argparse, because much function will call gettext in argpase(it's looks untidy).

vstinner
vstinner reviewed May 4, 2020
View reviewed changes
Comment thread Lib/gettext.py


import locale
import os

@vstinner vstinner May 4, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please write a separated PR for gettext, and keep this PR for test.support.

@shihai1991 shihai1991 May 4, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

OK, I created #19905 to do this job.

@vstinner
vstinner merged commit 975408c into python:master May 4, 2020
@bedevere-bot

bedevere-bot commented May 4, 2020

Copy link
Copy Markdown

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

Reviewers

@vstinner vstinner vstinner requested changes

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@shihai1991 @bedevere-bot @serhiy-storchaka @vstinner @the-knights-who-say-ni