◐ Shell
reader mode source ↗
Skip to content

Add localeconv function to locale module#4558

Merged
youknowone merged 30 commits into
RustPython:mainfrom
minhrongcon2000:fix/add-localeconv-function
Feb 28, 2023
Merged

Add localeconv function to locale module#4558
youknowone merged 30 commits into
RustPython:mainfrom
minhrongcon2000:fix/add-localeconv-function

Conversation

@minhrongcon2000

@minhrongcon2000 minhrongcon2000 commented Feb 24, 2023 •
edited by youknowone
Loading

Copy link
Copy Markdown
Contributor

This PR is to add localeconv function to locale module.

#3850

youknowone
youknowone requested changes Feb 24, 2023
View reviewed changes

@youknowone youknowone 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.

Hide comment

Thanks! I left a few comments about styles.

@minhrongcon2000 minhrongcon2000 force-pushed the fix/add-localeconv-function branch from c73148b to 09c750c Compare February 24, 2023 14:42
@minhrongcon2000 minhrongcon2000 requested review from fanninpm and youknowone and removed request for fanninpm February 24, 2023 16:41
@youknowone

youknowone commented Feb 25, 2023

Copy link
Copy Markdown
Member

test_setlocale_category is also successful!

10 hidden items Load more…
@youknowone

youknowone commented Feb 25, 2023

Copy link
Copy Markdown
Member

The windows failure is fine. But test_type failure on linux is... what's happened

@minhrongcon2000

minhrongcon2000 commented Feb 26, 2023

Copy link
Copy Markdown
Contributor Author

https://github.com/RustPython/RustPython/actions/runs/4271112901/jobs/7435359720#step:12:12102
@youknowone It seems like the format() function is not implemented with locale, so it returns an unformatted string, resulting in failed test case...

@youknowone youknowone added the z-ls-2023 Tag to track Line OSS Sprint 2023 label Feb 26, 2023
@minhrongcon2000

minhrongcon2000 commented Feb 26, 2023

Copy link
Copy Markdown
Contributor Author

@youknowone it turns out the locale code is inconsistent between platform, so after some fixes, it's normal now!

@minhrongcon2000 minhrongcon2000 force-pushed the fix/add-localeconv-function branch from 77af913 to 26f04e2 Compare February 28, 2023 05:53
youknowone
youknowone approved these changes Feb 28, 2023
View reviewed changes

@youknowone youknowone left a comment •
edited
Loading

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.

Hide comment

Thank you for hard working for this tricky issue. It will be a great base of the future locale works.

Maintaining note: Though a few tests regressed, I think this is the way to go forward with true locale support.
squash is desired when merging.

@youknowone youknowone merged commit af1d46f into RustPython:main Feb 28, 2023
@youknowone youknowone mentioned this pull request Mar 30, 2025
Closed
@youknowone youknowone mentioned this pull request Dec 25, 2025
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fanninpm fanninpm fanninpm left review comments

@youknowone youknowone youknowone approved these changes

Assignees

No one assigned

Labels

z-ls-2023

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@minhrongcon2000 @youknowone @fanninpm