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

Use numpy 1.16 by default #1501

Merged
merged 10 commits into from
May 27, 2019
Merged

Use numpy 1.16 by default #1501

merged 10 commits into from
May 27, 2019

Conversation

jenshnielsen
Copy link
Collaborator

Since its now available in both Conda and Pip form

@jenshnielsen
Copy link
Collaborator Author

This has uncovered a corner case bug in numpy reported here numpy/numpy#13089

@codecov
Copy link

codecov bot commented Mar 6, 2019

Codecov Report

Merging #1501 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #1501      +/-   ##
=========================================
- Coverage   73.82%   73.8%   -0.02%     
=========================================
  Files          92      92              
  Lines       10438   10434       -4     
=========================================
- Hits         7706    7701       -5     
- Misses       2732    2733       +1

@codecov
Copy link

codecov bot commented Mar 6, 2019 •
edited
Loading

Codecov Report

Merging #1501 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1501   +/-   ##
=======================================
  Coverage   72.18%   72.18%           
=======================================
  Files         112      112           
  Lines       12293    12293           
=======================================
  Hits         8874     8874           
  Misses       3419     3419

@WilliamHPNielsen
Copy link
Contributor

I'm slightly confused... The CI passed using numpy 16.2.1, but it was our CI that uncovered the corner case bug? Or did you discover it locally?

Is it safe to merge, or should we wait for this numpy/numpy#13092 to get in first?

@jenshnielsen
Copy link
Collaborator Author

@WilliamHPNielsen It is a hypothesis test that generates the issue. So it is not deterministic. We may see random failures if we merge this but they are unlikely to happen in real life.

@jenshnielsen
Copy link
Collaborator Author

We can also restrict the inputs to be more realistic

@WilliamHPNielsen
Copy link
Contributor

I see, thanks for explaining. I guess it's best to wait, then, depending a bit on how much work the restricting of inputs is. If it's a broad range of strategies in many tests (I guess we use linspace a lot), then it's not worth the bother, I think.

@astafan8
Copy link
Contributor

numpy/numpy#13092 was merged. Let's see what CI says now. If there are no problems, shall we merge? @QCoDeS/core

@jenshnielsen
Copy link
Collaborator Author

Yes, this was backported to numpy 1.16.3 which is released and now available in conda too

@jenshnielsen
Copy link
Collaborator Author

@QCoDeS/core I think this can land now if someone approves it

WilliamHPNielsen
WilliamHPNielsen approved these changes May 27, 2019
View reviewed changes
Copy link
Contributor

@WilliamHPNielsen WilliamHPNielsen left a comment

Choose a reason for hiding this comment

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

Let's move things forward

@jenshnielsen jenshnielsen merged commit cabc5c0 into microsoft:master May 27, 2019
giulioungaretti pushed a commit that referenced this pull request May 27, 2019
Generated gh-pages for commit cabc5c0
08d313b 
Merge: 5aabd25 31ad9a8
Author: Jens Hedegaard Nielsen <jenshnielsen@gmail.com>

    Merge pull request #1501 from jenshnielsen/fix/numpy_16
@jenshnielsen jenshnielsen deleted the fix/numpy_16 branch October 25, 2024 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WilliamHPNielsen WilliamHPNielsen WilliamHPNielsen 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.
3 participants
@jenshnielsen @WilliamHPNielsen @astafan8