Skip to content
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

Default locale logical compatibility with Python 3.12 #1054

Merged
merged 3 commits into from Apr 11, 2024

Conversation

liushuyu
Copy link
Contributor

@liushuyu liushuyu commented Feb 2, 2024

Description:

Related issue (if applicable): N/A

This pull request fixes the behavioural differences on Python 3.12, where if the default locale is unspecified, the new Python defaults to C.UTF-8 instead of en_US.UTF-8 on Unix systems.

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • No lint errors (use flake8)
  • 100% test coverage

Testing

Anyone can help test this source code as follows:

# Create a virtual environment to work in as follows:
python3 -m venv apprise

# Change into our new directory
cd apprise

# Activate our virtual environment
source bin/activate

# Install the branch
pip install git+https://github.com/caronc/apprise.git@<this.branch-name>

# Test out the changes with the following command:
apprise -t "Test Title" -b "Test Message" \
  <apprise url related to ticket>

@codecov-commenter
Copy link

codecov-commenter commented Feb 2, 2024

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 99.26%. Comparing base (31a4f2e) to head (8115a10).
Report is 34 commits behind head on master.

❗ Current head 8115a10 differs from pull request most recent head 729f53e. Consider uploading reports for the commit 729f53e to get more accurate results

Files Patch % Lines
apprise/AppriseLocale.py 0.00% 1 Missing and 1 partial ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1054      +/-   ##
==========================================
- Coverage   99.27%   99.26%   -0.02%     
==========================================
  Files         135      135              
  Lines       17600    17602       +2     
  Branches     3592     3593       +1     
==========================================
  Hits        17472    17472              
- Misses        119      120       +1     
- Partials        9       10       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

apprise/AppriseLocale.py Outdated Show resolved Hide resolved
@caronc
Copy link
Owner

caronc commented Apr 10, 2024

Ties to #1102; my only concern with this PR is it over-rides Python versions less than 3.12 which the test works fine for.

I think this PR may be a okay work-around for now but i'm not sure if fixing the language to us is always the solution (consider other countries). There must be a check next if lang == C; the question is what i next (how is it done in all previous versions)?

Consider this tweak but even then i still don't accept my own answer 🙂

# Based on patch already here...
import sys

if lang == 'C' and sys.version_info[0] == 3 and sys.version_info[1] == 12:
    # Alternatively sys.version_info[1] >=3
    lang = 'en_US'  # I still question the hard-coded assignment to english. 

There are discussions in the following spots that state that C.UTF-8 != en_US.UTF-8:

  • Stack Overflow
  • SuperUser: references Wikipedia (which i could not link/follow) stating that LANG=C is a means of disabling localization (not actually setting one).

I wonder if it would be better to just support C; store it and:

  • update Translation package to just no longer perform any translations at all. just return empty strings when asked upon.
  • Displaying 1 warning message (identifying working environment could not be detected).

That would handle the ambiguity i can't seem to let go of 😉 .

In the past, when building on different operating systems in different countries (Pre Python 3.12) when C is set, i'm curious what the strategies are taken, and what the language flips to? Perhaps it actually does hard-code it to en_us.utf-8 ?

Edit: Revisiting the code and the change made, i realize i'm already choosing en as a hard-coded language 👀 🤦‍♂️ . So with that, I just made one small patch to reference the same constant. I'll have to revisit my own rant another day.

@caronc
Copy link
Owner

caronc commented Apr 11, 2024

Thank you @liushuyu for this PR, and sorry i took so long to merge it in. I do appreciate it! Thank you for also letting me have my "old man yelling at cloud" moment! 🙂
image

@caronc caronc merged commit fa4a5a6 into caronc:master Apr 11, 2024
13 checks passed
@liushuyu
Copy link
Contributor Author

liushuyu commented May 6, 2024

Sorry for the late reply.

Thank you @liushuyu for this PR, and sorry i took so long to merge it in. I do appreciate it! Thank you for also letting me have my "old man yelling at cloud" moment! 🙂

No problem! I could understand the underlying Python change must be a headache for the project.

In the past, when building on different operating systems in different countries (Pre Python 3.12) when C is set, i'm curious what the strategies are taken, and what the language flips to? Perhaps it actually does hard-code it to en_us.utf-8 ?

Edit: Revisiting the code and the change made, i realize i'm already choosing en as a hard-coded language 👀 🤦‍♂️ . So with that, I just made one small patch to reference the same constant. I'll have to revisit my own rant another day.

Just add to this, I think the new Python behavior is more "correct": many other languages use C.utf-8 as the fallback locale to signify that the current environment does not currently have a locale set (but still allows the program to read/interpret/pass valid Unicode data through glibc when doing system I/O).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants