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

MAINT: additional 1.8.1 backports/adjustments #16146

Merged

Conversation

tylerjereddy
Copy link
Contributor

@tylerjereddy tylerjereddy added the maintenance Items related to regular maintenance tasks label May 8, 2022
@tylerjereddy tylerjereddy added this to the 1.8.1 milestone May 8, 2022
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

The backported PRs all look reasonable. There are a lot of failures though. The ones due to a distutils deprecation warning can be ignored. These ones may be due to the UNU.RAN update (not sure, @tirthasheshpatel do you remember?)

RuntimeWarning: invalid value encountered in multiply

@tirthasheshpatel
Copy link
Member

These ones may be due to the UNU.RAN update

No, I don't think the failures are related to UNU.RAN. The runtime warnings might be because of a change in NumPy. The prerelease_deps_coverage_64bit_blas uses the latest NumPy (1.23.0dev0).

@tylerjereddy
Copy link
Contributor Author

For the failures @rgommers is worried about, isn't that just the thing that @seberg was shimming for bleeding-edge NumPy in gh-16152? It looks like he is even working on the same file over there scipy/stats/tests/test_distributions.py.

I also see a comment that the exact nature of the warnings is still subject to change, but I don't mind if we want to try backporting that when it is ready before we proceed here?

seberg and others added 2 commits May 10, 2022 19:51
The scalar math in NumPy gives a slightly different warning then the
ufunc machinery.  In this particular case, the ufunc machinery is now
used, rather than the array case.

That might change again (the ufunc path is quite a bit slower), but
in general the warning given by the ufunc path is also much more useful
so just fortify the tests.

Note, one test failure remains due to: numpy/numpy#21481
@tylerjereddy
Copy link
Contributor Author

Ok, I dealt with the merge conflicts to pull in the new-NumPy shims from Sebastian and updated the relnotes.

Still, there are more failures to scan over I think, hopefully this reduces the count a bit more though.

@tylerjereddy
Copy link
Contributor Author

The prerelease job is timing out now, making it harder to assess remaining failures--I'll try a restart or two.

@tylerjereddy
Copy link
Contributor Author

For the NumPy prerelease failures, ignoring the distutils deprecation errors, we still have failures in:

  • TestSOSFreqz.test_sosfreqz_design_ellip
  • TestCorrelateComplex.test_rank0[complex256]
    • I think @seberg is aware of this one--is it safe to ignore TypeError: unsupported operand type(s) for *: 'complex' and 'numpy.float128'?
  • TestSparseFunctions.test_scale_rows_and_cols
    • PendingDeprecationWarning: the matrix subclass is not the recommended way to represent matrices or deal with linear algebra (see https://docs.scipy.org/doc/numpy/user/numpy-for-matlab-users.html). Please adjust your code to use regular ndarray. safe to ignore for backporting since it is just a pending deprecation?
    • @rossbar thoughts? you've touched code around here lately I think?
  • TestTruncnorm.test_moments
    • small floating point delta: 5.8718046 vs. 5.871785 @mdhaber I think a 32-bit xfail was added recently, should we worry about this one?

@rkern @tupui @tirthasheshpatel looks like there may be some disagreement on the licensing backports (gh-16155, gh-16158)--suggestions?

@rkern
Copy link
Member

rkern commented May 12, 2022

I believe that this commit should be reverted, yes. The rest of the PR is fine.

@tupui
Copy link
Member

tupui commented May 12, 2022

I will make the fix for the licensing. But fine with me to not block the release for that.

@WarrenWeckesser
Copy link
Member

If you need it to fix test failures in the 1.8.1 release, then sure, backporting is fine. It is just a one-line fix of a test (three if you include the comments), so it should be straightforward to include it.

@tupui
Copy link
Member

tupui commented May 14, 2022

@tylerjereddy gh-16163 is in. I believe this is good now.

@tylerjereddy
Copy link
Contributor Author

@tylerjereddy #16163 is in. I believe this is good now.

Thanks! Yeah, main delay is just that I'm still recovering from some kind of virus, hopefully I can resume here soon.

@tupui
Copy link
Member

tupui commented May 14, 2022

oh no worries, take care of yourself first!

tylerjereddy and others added 6 commits May 15, 2022 15:24
…st equal'.

The constant upper bound of the dB gain in the stop band
is increased to -150*(1-1e-12), effectively converting
the test into 'less or almost equal'.
MAINT: better/corrected UNU.RAN licensing information
This change can be reverted once Pip releases its next version with
a fix for pypa/pip#11116.

At the moment all our Azure builds are failing with errors like:
```
ERROR: Some build dependencies for file:///D:/a/1/s conflict with the backend dependencies:
numpy==1.21.4 is incompatible with numpy==1.18.5; python_version=='3.8' and (platform_machine!='arm64' or platform_system!='Darwin') and platform_machine!='aarch64' and platform_python_implementation != 'PyPy'.
```

[skip github]
rgommers and others added 2 commits May 15, 2022 15:50
…#16175)

Closes scipygh-16174. Note that this will likely be fixed in NumPy,
but the test is fine written this way and it fixes an annoying CI
failure, so we can leave the test like this.

[skip azurepipelines]
@tylerjereddy
Copy link
Contributor Author

tylerjereddy commented May 15, 2022

I've updated to backport everything I think I need here, along with another release notes update.

I'll wait for CI because we had so many failures before. My current plan is to allow for the following types of test suite failures with pre-release NumPy:

  • PendingDeprecationWarning: the matrix subclass is not the recommended way to represent matrices (this was primarily in TestSparseFunctions.test_scale_rows_and_cols) -- I do not see objections above to proposal of allowing these warnings to continue on this release branch
  • TestTruncnorm.test_moments differs at the 4th decimal position with pre-release NumPy, and the test already has a 32-bit xfail for causing trouble
  • distutils deprecation warnings (Ralf already excused those above)

So, if I just see those failures I'll be tempted to self-merge. If I see additional failures with pre-release NumPy I may have to check those again.

@tylerjereddy
Copy link
Contributor Author

tylerjereddy commented May 16, 2022

There are only two failing CI jobs left:

  1. the circleci asv benchmark timeout, which we already agreed to ignore when the first thing we tried to fix that had no effect
  2. the prerelease deps job has 3 test failures and 2 errors left--they all involve either:
    - distutils deprecations
    - matrix subclass pending deprecations
    - or other cases I mentioned in my comment above

@tylerjereddy tylerjereddy merged commit 52b5069 into scipy:maintenance/1.8.x May 16, 2022
@tylerjereddy tylerjereddy deleted the treddy_181_more_backporting branch May 16, 2022 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Items related to regular maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants