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

Remove unnecessary setting of locale::global #1636

Merged

Conversation

danieldresser-ie
Copy link
Contributor

We used to need to set the global locale before calling something that used OIIO parsing functions. This is no longer the case ( as of OIIO-1.9, and we now require 2.3 ), so we can now remove these calls.

It came up for me because this code seems to interact strangely with locale in Python - I think this is because something synchs the C++ locale across to Python, but if locale::global hasn't been called in C++ yet, and the locale has only been set in Python, oldlocale doesn't get set to the Python locale ... I could try to figure out more of what's going on, but the simple answer is just to remove this code that is no longer needed. LG though it was reasonable when I asked on the dev list: https://lists.aswf.io/g/osl-dev/message/5081

A potential follow-up would be to move the lock_guard inside parse, since there's a comment indicating that the locale was the reason this hadn't been done.

Checklist:

  • I have read the contribution guidelines.
  • I have previously submitted a Contributor License Agreement.
  • I have updated the documentation, if applicable.
  • I have ensured that the change is tested somewhere in the testsuite (adding new test cases if necessary).
  • My code follows the prevailing code style of this project.

Signed-off-by: Daniel Dresser <danield@image-engine.com>
Copy link
Collaborator

@lgritz lgritz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@lgritz lgritz merged commit b064b25 into AcademySoftwareFoundation:main Jan 12, 2023
lgritz pushed a commit to lgritz/OpenShadingLanguage that referenced this pull request Jan 12, 2023
…on#1636)

Signed-off-by: Daniel Dresser <danield@image-engine.com>
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

2 participants