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

Numpy Deprecated Code Replacement #3126

Merged
merged 5 commits into from Jan 18, 2022

Conversation

shubham1206agra
Copy link
Contributor

  • Disabled use of Deprecated Numpy API use by defining kwarg in Extension in setup.py.in
define_macros=[('NPY_NO_DEPRECATED_API', 'NPY_1_7_API_VERSION')]

which will turn it in c++ into

#define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION
  • Changing direct use of members of PyArrayObject to function based access
  1. X.data changes to PyArray_DATA(X)
  2. X.shape changes to PyArray_SHAPE(X)
  3. X.flags changes to PyArray_FLAGS(X)
  • Changed NPY_OWNDATA to NPY_ARRAY_OWNDATA
  • Changed the if logic
if not X.flags.c_contiguous or (not X.flags.owndata and not isWin)

to

if not (flags & numpy.NPY_ARRAY_C_CONTIGUOUS) or (not (flags & numpy.NPY_ARRAY_OWNDATA) and not isWin)

Copy link
Member

@rcurtin rcurtin left a comment

Choose a reason for hiding this comment

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

This is awesome! Thank you so much for taking the time to fix this issue. I really like little code maintenance PRs like this. 💯 I tested the build myself and indeed, no more warnings. 🚀

I only have a couple little comments relating to style, and then we should definitely merge this. 👍

src/mlpack/bindings/python/print_input_processing.hpp Outdated Show resolved Hide resolved
src/mlpack/bindings/python/mlpack/arma_numpy.pyx Outdated Show resolved Hide resolved
Removing syntax error
Added removed newline at end of file
@shubham1206agra
Copy link
Contributor Author

You may merge now.

Copy link
Member

@rcurtin rcurtin left a comment

Choose a reason for hiding this comment

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

Thanks! I really appreciate this PR. Would you like to add an entry to CHANGELOG.md? 👍

Copy link

@mlpack-bot mlpack-bot bot left a comment

Choose a reason for hiding this comment

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

Second approval provided automatically after 24 hours. 👍

@rcurtin rcurtin merged commit eccd230 into mlpack:master Jan 18, 2022
@rcurtin
Copy link
Member

rcurtin commented Jan 18, 2022

Thanks @shubham1206agra!

@shubham1206agra shubham1206agra deleted the numpy-deprecated branch January 28, 2022 13:03
This was referenced Oct 14, 2022
@rcurtin rcurtin mentioned this pull request Oct 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants