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

Fix Numpy and Scipy deprecation warnings #163

Merged
merged 5 commits into from
Feb 29, 2024

Conversation

yanbomb
Copy link
Contributor

@yanbomb yanbomb commented Feb 26, 2024

Fix Numpy and Scipy deprecation warnings.

Changes:

  • numpy.product() -> numpy.prod()
  • array to float conversion for functions used by scipy.integrate.quad/quadn and scipy.optimize.brentq

Setup:

  • Python 3.12.2
  • Numpy 1.26.4
  • Scipy 1.12.0

Verification:

I ran pytest and got 573 passed tests; 0 failues; 43 skipped (also ran these tests; all passed); 94 warnings. The warning are related to: a) Silverman and Scott methods with multiples weights (92 warnings), b) datetime deprecation (does not seem to be related to KDEpy)

Note:
There is another instance of brentq in bw_selection.py that did not emit warnings during testing. Code was left as is, but more testing could eventually reveal additional deprecation warnings. The easy fix would be to wrap the function using a lambda function:

ff=lambda x:function(x)[0]
x, res = brentq(ff, 0, tol, args=args, full_output=True, disp=False)

@tommyod
Copy link
Owner

tommyod commented Feb 26, 2024

Thanks a lot! Changes look good. Could you run the linter too, so the CI pipeline passes? Probably just some whitespace

@yanbomb
Copy link
Contributor Author

yanbomb commented Feb 26, 2024

I made some changes, but this is all new to me so I hope that I correctly addressed your concerns. While introducing myself to pylint and PEP-8, I noticed that many locations may not comply with PEP-8 (mostly regarding naming conventions and unused variables).

P.S. The major issue was that the usage of lambdas is not permitted. That was a surprise to me...

@yanbomb
Copy link
Contributor Author

yanbomb commented Feb 27, 2024

Made changes that I hope will fix the formatting issues and ran the following checks which resulted in no error:

  • python -m black KDEpy -l 120 --check
  • python -m flake8 --show-source --ignore=F811,W293,W391,W292,W291,W504,W503,E231 --max-line-length=120 --exclude="*examples.py,testing.py,*kde.py" KDEpy
  • pytest KDEpy --doctest-modules --capture=sys

P.S. Sorry for the iterative process for something that should be trivial... I am learning something new here with flake8 and black.

@whitews
Copy link
Collaborator

whitews commented Feb 27, 2024

Hi @yanbomb

Looks like black is getting caught on the lack of empty lines before & after the nested functions. She's a stickler for the rules ;)

An example from the diff:

@@ -54,12 +54,14 @@
 
         if function.finite_support:
             a, b = -function.support, function.support
         else:
             a, b = -5 * function.var, 5 * function.var
+
         def function_float(x):
             return function(x)[0]
+
         integral, _ = quad(function_float, a=a, b=b)
         assert np.isclose(integral, 1)

Dunno why the Windows tests are passing, but that should help you get the PEP8 stuff cleared up.

@yanbomb
Copy link
Contributor Author

yanbomb commented Feb 27, 2024

I finally got the formatting correct! Thank you @tommyod for your patience and @whitews for the hint.
P.S. That was my first PR...

@whitews
Copy link
Collaborator

whitews commented Feb 27, 2024

Nice! I triggered the tests and it all looks good, I'll leave for Tommy to merge.

@yanbomb
Copy link
Contributor Author

yanbomb commented Feb 27, 2024

Should I click the "close with comment" button to close this PR?

@whitews
Copy link
Collaborator

whitews commented Feb 27, 2024

Nope, you're all good. Tommy will have the final say & merge. He may want to bump the version for a new release to PyPI.

@tommyod
Copy link
Owner

tommyod commented Feb 27, 2024

Great job on your first PR @yanbomb . Thanks for the help @whitews !

I think you're all set. I'll merge later today or tomorrow, then I will bump versions at the same time and.

@tommyod tommyod merged commit fcc38dd into tommyod:master Feb 29, 2024
15 checks passed
tommyod added a commit that referenced this pull request Sep 6, 2024
* Fix Numpy and Scipy deprecation warnings (#163)

* fix numpy deprecation (product to prod)

* fix numpy deprecation (brentq)

* fix numpy deprecation (quad/nquad)

* changes to meet PEP-8

* Fix flake8 and black formatting errors

* Bump version to 1.1.9 (#164)

Co-authored-by: tommyod <tommy.odland>

* Update build.yml with pypi API token (#165)

* Update build.yml with pypi API token

* Remove push condition

* Replaced numpy.asfarray with numpy.asarray using the proper dtype. (#169)

* Replaced numpy.asfarray with numpy.asarray using the proper dtype.

* Newer version of numpy return np.True_ instead of True. Changed this in two doctests.

* Changed to correct dtype from int32 to long.
Changed doctest to use assert

---------

Co-authored-by: skrylow <[email protected]>

* Version 1.1.10 with numpy 2.0 support (#171)

---------

Co-authored-by: yanbomb <[email protected]>
Co-authored-by: SergejKr <[email protected]>
Co-authored-by: skrylow <[email protected]>
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