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

Check numpy version before passing rcond to numpy.linalg.lstsq #1670

Merged
merged 3 commits into from
Jul 29, 2019

Conversation

mliu49
Copy link
Contributor

@mliu49 mliu49 commented Jul 25, 2019

Motivation or Problem

#1659 added the rcond argument to calls to numpy.linalg.lstsq. A side-effect is that we broke compatibility with numpy versions prior to 1.14.

Original plan was to simply increase our numpy version requirement, but after thinking about it a bit more, this change does not seem like sufficient reason to do so.

Description of Changes

This PR adds automatic determination of the appropriate value for rcond depending on the numpy version.

It also updates and futurizes utilities.py.

Testing

Will test in an environment with an older version of numpy. Travis should be sufficient to test that it works in newer versions of numpy.

@mliu49 mliu49 requested a review from amarkpayne July 25, 2019 21:42
@codecov
Copy link

codecov bot commented Jul 25, 2019

Codecov Report

Merging #1670 into master will increase coverage by <.01%.
The diff coverage is 25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1670      +/-   ##
==========================================
+ Coverage   41.88%   41.88%   +<.01%     
==========================================
  Files         176      176              
  Lines       29368    29369       +1     
  Branches     6059     6059              
==========================================
+ Hits        12301    12302       +1     
  Misses      16188    16188              
  Partials      879      879
Impacted Files Coverage Δ
rmgpy/data/kinetics/groups.py 15.4% <25%> (+0.25%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1369f9e...7966b16. Read the comment docs.

mliu49 added 3 commits July 28, 2019 10:43
Prior to numpy 1.14, numpy.linag.lstsq does not accept rcond=None
and will raise a TypeError.

In numpy 1.14 and after, None requests the new default behavior.
@mliu49
Copy link
Contributor Author

mliu49 commented Jul 29, 2019

I've confirmed that this works with numpy 1.11

@amarkpayne amarkpayne merged commit 69122e6 into master Jul 29, 2019
@amarkpayne amarkpayne deleted the numpy branch July 29, 2019 19:02
@mliu49 mliu49 mentioned this pull request Dec 16, 2019
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.

2 participants