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

Avoid erroring when OMP_NUM_THREADS is empty string #472

Merged
merged 4 commits into from
Feb 10, 2024

Conversation

phofl
Copy link
Contributor

@phofl phofl commented Feb 2, 2024

unsetting the variable like this is not uncommon but this will raise here unfortunately

This circumvents the issue

@phofl
Copy link
Contributor Author

phofl commented Feb 8, 2024

@FrancescAlted gentle ping

thoughts here?

numexpr/utils.py Outdated
@@ -152,7 +152,8 @@ def _init_num_threads():
# actual number of threads used.
if 'NUMEXPR_NUM_THREADS' in os.environ:
Copy link

Choose a reason for hiding this comment

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

Would be good to fix this one as well, and any others that blindly call int on an environmental variable. Would it be more pythonic to try/except and then reraise (or warn) with an explanation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think an empty string should be fine, so just ignoring it seems good to me

Copy link
Contributor

@FrancescAlted FrancescAlted left a comment

Choose a reason for hiding this comment

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

This sounds good to me. When we are that, would you like to add the suggestion by @bashtage ?

@phofl
Copy link
Contributor Author

phofl commented Feb 9, 2024

Updated

@FrancescAlted
Copy link
Contributor

Unfortunately test suite is not passing with the latest commit.

@phofl
Copy link
Contributor Author

phofl commented Feb 10, 2024

should be fixed now

@FrancescAlted FrancescAlted merged commit b6b4f7c into pydata:master Feb 10, 2024
4 checks passed
@FrancescAlted
Copy link
Contributor

Looks good. Thanks @phofl !

@phofl phofl deleted the omp_num_threads branch February 10, 2024 12:05
This pull request was closed.
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.

3 participants