Skip to content

MNT Replaces numpy alias with builtin typse#17687

Merged
adrinjalali merged 2 commits intoscikit-learn:masterfrom
thomasjpfan:replace_deprecated_np_items
Jun 24, 2020
Merged

MNT Replaces numpy alias with builtin typse#17687
adrinjalali merged 2 commits intoscikit-learn:masterfrom
thomasjpfan:replace_deprecated_np_items

Conversation

@thomasjpfan
Copy link
Copy Markdown
Member

@thomasjpfan thomasjpfan commented Jun 23, 2020

This PR was generated by:

$ find . -name "*.py" -print0 | xargs -0 sed -i "s/\bnp.int\b/int/g"
$ find . -name "*.py" -print0 | xargs -0 sed -i "s/\bnp.float\b/float/g"
$ find . -name "*.py" -print0 | xargs -0 sed -i "s/\bnp.bool\b/bool/g"

Numpy alias to python builtins are deprecated in 1.20

CC: @adrinjalali I hear you like the command line :)

glemaitre
glemaitre approved these changes Jun 24, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

LGTM. What is the reason to make this change?

@rth
Copy link
Copy Markdown
Member

rth commented Jun 24, 2020

A similar fix was done to numpy earlier numpy/numpy#14901 and they mostly use np.int_ etc. versions. Personally I think it's better to use np.float32 or np.float64 explicitly rather than float. Not sure if np.int_ is better than int or not.

@alfaro96
Copy link
Copy Markdown
Member

alfaro96 commented Jun 24, 2020

LGTM. What is the reason to make this change?

Now that the numpy and scipy wheels are fetched from the Anaconda index (see PR #17672), numpy==1.20.dev0 is used for the CRON jobs (travis and Linux_Nightly pylatest_pip_scipy_dev). Since numpy aliases are deprecated from this version, this will raise to the CRON jobs to fail.

For instance, see this job.

@glemaitre
Copy link
Copy Markdown
Member

glemaitre commented Jun 24, 2020

Thanks @alfaro96 it makes sense then.

adrinjalali
adrinjalali approved these changes Jun 24, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

I agree that np.float32 or np.float64 are much better, but that's more of another thread of issues and improvements that's going on, and I think deserve their own PRs. I'm happy with this one as is.

@adrinjalali adrinjalali merged commit 7cc0177 into scikit-learn:master Jun 24, 2020
@alfaro96 alfaro96 mentioned this pull request Jun 24, 2020
Merged
rubywerman pushed a commit to MLH-Fellowship/scikit-learn that referenced this pull request Jun 24, 2020
@thomasjpfan @rubywerman
MNT Replaces numpy alias with builtin typse (scikit-learn#17687)
6aef553 
* MNT Replaces numpy alias with builtin typse

* STY Lint error
@thomasjpfan thomasjpfan mentioned this pull request Jun 25, 2020
Merged
@eric-wieser
Copy link
Copy Markdown

eric-wieser commented Jun 30, 2020

xref numpy/numpy#14882

jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020
@thomasjpfan @jayzed82
MNT Replaces numpy alias with builtin typse (scikit-learn#17687)
1506cf6 
* MNT Replaces numpy alias with builtin typse

* STY Lint error
@alfaro96 alfaro96 mentioned this pull request Jan 25, 2021
Merged
@xuefeng-xu xuefeng-xu mentioned this pull request Aug 11, 2023
Closed
@DimitriPapadopoulos DimitriPapadopoulos mentioned this pull request Nov 27, 2023
Closed
@glemaitre glemaitre mentioned this pull request Aug 17, 2024
Closed
@jameslamb jameslamb mentioned this pull request Jan 26, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@adrinjalali adrinjalali adrinjalali approved these changes

@glemaitre glemaitre glemaitre approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@thomasjpfan @rth @alfaro96 @glemaitre @eric-wieser @adrinjalali