-
-
Notifications
You must be signed in to change notification settings - Fork 297
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
Py2/Py3 compat: Use "gettext.install(..., unicode=True)" in Python 2. #3
Conversation
`unicode` is no longer a valid keyword for `gettext.install()` in Python 3 but we need it on Python 2. This is in continuation of https://trac.osgeo.org/grass/changeset/74307 which was a fix for: https://trac.osgeo.org/grass/ticket/3790 Fixes https://trac.osgeo.org/grass/ticket/3838
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I said in https://trac.osgeo.org/grass/ticket/3790, I think https://trac.osgeo.org/grass/changeset/74307 should be reverted because:
- It is still unclear if there is something left in https://trac.osgeo.org/grass/ticket/3838 which is caused by 74307 (https://trac.osgeo.org/grass/ticket/3838#comment:12)
- It does not take into account https://trac.osgeo.org/grass/ticket/1739 and https://trac.osgeo.org/grass/ticket/2425 and it is not following the recommendations for libraries.
- It seems that it is trying to fix something else, so the question is if other change is not more appropriate to solve that (https://trac.osgeo.org/grass/ticket/3790#comment:5, https://trac.osgeo.org/grass/ticket/3772)
I don't know how these changes would be applicable with 74307 reverted. It is okay as a quick fix for part of 3838, but I would prefer this being revisited from the beginning, esp. if we are trying to make the grass
package behave like a generally usable Python package.
This PR contains the patch that @marisn confirmed that solves 3838. WRT to the traceback that is mentioned on comment 12, I still haven't heard from @marisn, but I did try to reproduce it and I could not. If someone can offer clear instructions on how to reproduce the issue, I will be happy to look into it. BTW, the traceback is incomplete, so it is not easy to understand what is the problem, but from what I can tell, it does not seem to be related to gettext. That being said, the other points should probably be discussed on a separate issue or on the ML. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works fine and makes sense. I can't reproduce the problem either.
Please merge the PR. |
Change of input arguments, and adding test. OSGeo#3 Co-Authored-By: dekanluc <dekanluc@users.noreply.github.com>
* CI: Switch Travis to Ubuntu 22.04 (jammmy) Default Python version reported by Travis for 20.04 is 3.7.13. While that's still a supported version for 8.3 release if it would be released now, 2023-06-27 is end of support for 3.7, so it seems safe to not test the main branch with 3.7 at this point. Ubuntu 22.04 in Travis uses Python 3.10 by default according to the documentation. * Update wxgtk pkg to 4.0 * Replace custom install list by apt.txt file from GitHub Actions * Change shebang to use bash instead of POSIX sh (#3) * CI(travis): Add --with-pdal in linux.script.sh * CI(travis): Remove sudo: required as it has no effect * CI(travis): Fix warning os and dist missing from root * CI(travis): Fix warning matrix is an alias for jobs * CI(travis): Run make with make -j $(nproc) * CI(travis): Show MAKEFLAGS at start of script * CI(travis): use c++17 standard * CI(travis): Remove duplicated dist key in include array * CI(travis): Remove invalid --with-python from configure flags * CI(travis): Remove unused codecov upload * CI(travis): Remove irc notification * CI(travis): Add -Werror and -fPIC to CFLAGS and CXXFLAGS on make call * CI(travis): Limit runs on branches to main and release branches * CI(travis): Add -Wfatal-errors * db: Fix -Wdeprecated-non-prototype in describe.c for clang builds * CI(travis): Add --no-keep-going in MAKEFLAGS to stop on errors * db: Fix -Wdeprecated-non-prototype in odbc driver's fetch.c for clang builds * db: Fix -Wdeprecated-non-prototype in odbc driver's listtab.c for clang builds --------- Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com>
…o#3002) * CI: Switch Travis to Ubuntu 22.04 (jammmy) Default Python version reported by Travis for 20.04 is 3.7.13. While that's still a supported version for 8.3 release if it would be released now, 2023-06-27 is end of support for 3.7, so it seems safe to not test the main branch with 3.7 at this point. Ubuntu 22.04 in Travis uses Python 3.10 by default according to the documentation. * Update wxgtk pkg to 4.0 * Replace custom install list by apt.txt file from GitHub Actions * Change shebang to use bash instead of POSIX sh (OSGeo#3) * CI(travis): Add --with-pdal in linux.script.sh * CI(travis): Remove sudo: required as it has no effect * CI(travis): Fix warning os and dist missing from root * CI(travis): Fix warning matrix is an alias for jobs * CI(travis): Run make with make -j $(nproc) * CI(travis): Show MAKEFLAGS at start of script * CI(travis): use c++17 standard * CI(travis): Remove duplicated dist key in include array * CI(travis): Remove invalid --with-python from configure flags * CI(travis): Remove unused codecov upload * CI(travis): Remove irc notification * CI(travis): Add -Werror and -fPIC to CFLAGS and CXXFLAGS on make call * CI(travis): Limit runs on branches to main and release branches * CI(travis): Add -Wfatal-errors * db: Fix -Wdeprecated-non-prototype in describe.c for clang builds * CI(travis): Add --no-keep-going in MAKEFLAGS to stop on errors * db: Fix -Wdeprecated-non-prototype in odbc driver's fetch.c for clang builds * db: Fix -Wdeprecated-non-prototype in odbc driver's listtab.c for clang builds --------- Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com>
unicode
is no longer a valid keyword forgettext.install()
in Python 3but we need it on Python 2.
This is in continuation of https://trac.osgeo.org/grass/changeset/74307
which was a fix for: https://trac.osgeo.org/grass/ticket/3790
Fixes https://trac.osgeo.org/grass/ticket/3838