Skip to content

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

Merged
amarkpayne merged 3 commits intomasterfrom
numpy
Jul 29, 2019
Merged

Check numpy version before passing rcond to numpy.linalg.lstsq#1670
amarkpayne merged 3 commits intomasterfrom
numpy

Conversation

@mliu49
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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