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

support for py3.8 and np1.18 #631

Merged
merged 6 commits into from Jul 27, 2020
Merged

support for py3.8 and np1.18 #631

merged 6 commits into from Jul 27, 2020

Conversation

sbailey
Copy link
Contributor

@sbailey sbailey commented Jul 24, 2020

This PR cleans up some warnings from python 3.8 + numpy 1.18.5:

  • numpy is getting more picky about not using where on scalars, thus necessitating
    more verbose if np.isscalar(x) ... else ... code.
  • avoid calling np.median on empty slices (previously that would return a NaN, so
    still using that if the slice was empty).

Tests pass at NERSC using both the current desi environment with python/3.6 + numpy/1.16.4 and a test python/3.8 + numpy/1.18.5 environment, but I'm not sure if the tests cover the Gaia NaN and QA empty slice cases so please take a look.

@sbailey sbailey requested a review from geordie666 July 24, 2020 21:50
Copy link
Contributor

@geordie666 geordie666 left a comment

Choose a reason for hiding this comment

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

I ran some "typical" target/cmx cuts and QA cases in this branch and didn't trigger any errors. It's always possible, of course, that there are some cases we don't "typically" encounter, or some malicious cases I didn't think to run.

If we encounter some fragility in the future, we should fix the code and add a unit test to catch the specific failure mode. But, for now, I'm happy to approve this as-is.

@sbailey: I wasn't sure if you wanted to leave the:

warning.filterwarnings('error', '.*Calling nonzero on 0d arrays*')

set-ups in test_cmx.py and test_cuts.py. I'd probably remove them, as they seem specific to your test cases?

@sbailey
Copy link
Contributor Author

sbailey commented Jul 27, 2020

Thanks @geordie666 . In this case I do want to leave the "warnings as errors" for the tests, since I think if we accidentally do re-introduce this warning we want to fix it since it is triggered by a deprecated "feature" that will change in the future. We can turn it back off if we hit a legitimate case where that "nonzero on 0d arrays" warning is triggered that we don't want to work around (I'm not sure what that case would be...)

@sbailey sbailey merged commit e17821a into master Jul 27, 2020
@geordie666 geordie666 deleted the py3.8 branch August 6, 2020 21:00
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