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.

Sign up for GitHub

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

Jump to bottom

Compatibility with NumPy 1.20+ #48935

Closed
wants to merge 21 commits into from
Closed

Compatibility with NumPy 1.20+ #48935

wants to merge 21 commits into from

Conversation

bnavigator
Copy link
Contributor

@google-ml-butler google-ml-butler bot added the size:M CL Change Size: Medium label May 6, 2021
@google-cla google-cla bot added the cla: yes label May 6, 2021
@gbaned gbaned self-assigned this May 6, 2021
@gbaned gbaned added this to Assigned Reviewer in PR Queue via automation May 6, 2021
@bnavigator bnavigator marked this pull request as ready for review May 6, 2021 13:35
@gbaned gbaned requested a review from mihaimaruseac May 6, 2021 15:01
@mihaimaruseac mihaimaruseac added the kokoro:force-run Tests on submitted change label May 6, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 6, 2021
@bhack
Copy link
Contributor

bhack commented May 12, 2021

Just to notify, another user has submitted #49008

PR Queue automation moved this from Assigned Reviewer to Approved by Reviewer May 12, 2021
@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels May 12, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label May 12, 2021
@gbaned gbaned removed the ready to pull PR ready for merge process label May 27, 2021
@gbaned
Copy link
Contributor

gbaned commented May 27, 2021

@bnavigator Can you please resolve conflicts? Thanks!

@gbaned gbaned requested a review from mihaimaruseac May 28, 2021 12:48
@gbaned gbaned added the awaiting review Pull request awaiting review label Jun 9, 2021
@gbaned
Copy link
Contributor

gbaned commented Jun 10, 2021

@bnavigator Can you please resolve conflicts? Thanks!

@gbaned gbaned removed the awaiting review Pull request awaiting review label Jun 10, 2021
@bnavigator
Copy link
Contributor Author

@gbaned: I resolved conflicts. But you developers clearly stick to the pinning strategy

See also
fe2fad5#r52001636

@mihaimaruseac
Copy link
Collaborator

The mentioned commit is just a step in getting this handled.

We will keep pinning dependencies in our CI, but setup.py would have broader ranges

@bnavigator
Copy link
Contributor Author

I suggest, you make a PR and if it is merged before this one, I will happily revert 9a4642f.

@bnavigator
Copy link
Contributor Author

Could someone please trigger a kokoro run? @bhack, @mihaimaruseac?

@gbaned gbaned added the kokoro:force-run Tests on submitted change label Sep 2, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Sep 2, 2021
@terrymyc terrymyc mentioned this pull request Sep 3, 2021
Merged
@njzjz njzjz mentioned this pull request Sep 14, 2021
Closed
@google-ml-butler google-ml-butler bot added the awaiting review Pull request awaiting review label Sep 23, 2021
@mihaimaruseac
Copy link
Collaborator

Sorry, I was OOO. It seems now we have wide ranges of deps in setup.py and this causes conflicts.

@bnavigator
Copy link
Contributor Author

Merge conflict resolved.

@google-ml-butler google-ml-butler bot added kokoro:force-run Tests on submitted change ready to pull PR ready for merge process labels Oct 5, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Oct 5, 2021
@gbaned gbaned removed the awaiting review Pull request awaiting review label Oct 6, 2021
@penpornk penpornk added the kokoro:force-run Tests on submitted change label Oct 18, 2021
@kokoro-team kokoro-team removed the kokoro:force-run Tests on submitted change label Oct 18, 2021
@h-vetinari h-vetinari mentioned this pull request Nov 1, 2021
Closed
5 tasks
@gbaned gbaned added ready to pull PR ready for merge process and removed ready to pull PR ready for merge process labels Nov 3, 2021
@svenstaro
Copy link
Contributor

Can't this be closed now according to #52380?

@bnavigator
Copy link
Contributor Author

Yeah, #47957 "fixed" the np.prod error between TF 2.5 and 2.6 by catching the Exception correctly and not returning a constant. 0f8fde4 changes it to not run into the exception.

The changes in the requirements specifiers, the wrong comment about numpy install order and the wrong float shape tuples are still something to consider.

@svenstaro
Copy link
Contributor

Alright, fair enough.

@mihaimaruseac mihaimaruseac mentioned this pull request Nov 11, 2021
Closed
@wei-v-wang
Copy link
Contributor

Please consider reproducers in #52657 closed this as this PR is supposed to address such issues.

@copybara-service copybara-service bot closed this in 679d1e6 Dec 7, 2021
PR Queue automation moved this from Approved by Reviewer to Closed/Rejected Dec 7, 2021
@google-ml-butler google-ml-butler bot removed the ready to pull PR ready for merge process label Dec 7, 2021
@mihaimaruseac
Copy link
Collaborator

We now merged most of this PR. Disabled tests will be fixed ahead in the near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgommers rgommers rgommers approved these changes

@shoyer shoyer shoyer left review comments

@joshua-cogliati-inl joshua-cogliati-inl joshua-cogliati-inl left review comments

@mihaimaruseac mihaimaruseac mihaimaruseac approved these changes

@bhack bhack bhack approved these changes

@kkimdev kkimdev Awaiting requested review from kkimdev

@rohan100jain rohan100jain Awaiting requested review from rohan100jain rohan100jain is a code owner

Assignees

@gbaned gbaned

Labels
cla: yes size:M CL Change Size: Medium
Projects
PR Queue
  
Closed/Rejected
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet