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

Update NumPyPrinter to give the minimum over the first axis. #18774

Merged
merged 5 commits into from
Mar 5, 2020
Merged

Update NumPyPrinter to give the minimum over the first axis. #18774

merged 5 commits into from
Mar 5, 2020

Conversation

sbt4104
Copy link
Contributor

@sbt4104 sbt4104 commented Mar 4, 2020

References to other Issues or PRs

Fixes #18770
Fixes #18742

Brief description of what is fixed or changed

Added axis=0 at the end of the _print_Min() and _print_Max() to give minimum/maximum over the first axis of an array.
amin((x + 1,0.1x + 3,0.5x + 1)) got updated to amin((x + 1,0.1x + 3,0.5x + 1), axis=0).

Other comments

Please suggest changes.

Release Notes

  • printing
    • Fix lambdify with Min/Max for arrays of more than one dimension

@sympy-bot
Copy link

sympy-bot commented Mar 4, 2020

Hi, I am the SymPy bot (v157). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

Your release notes are in good order.

Here is what the release notes will look like:

  • printing
    • Fix lambdify with Min for arrays of more than one dimension (#18774 by @sbt4104)

This will be added to https://github.com/sympy/sympy/wiki/Release-Notes-for-1.6.

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

#### References to other Issues or PRs
Fixes #18770

#### Brief description of what is fixed or changed

Added axis=0 at the end of the _print_Min() and _print_Max() to give minimum/maximum over the first axis of an array.
amin((x + 1,0.1*x + 3,0.5*x + 1)) got updated to amin((x + 1,0.1*x + 3,0.5*x + 1), axis=0).

#### Other comments 
Please suggest changes.

### Release Notes
<!-- BEGIN RELEASE NOTES -->
* printing
    - Fix lambdify with `Min` for arrays of more than one dimension
<!-- END RELEASE NOTES -->

Update

The release notes on the wiki have been updated.

sympy/printing/pycode.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Mar 4, 2020

Codecov Report

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

@@              Coverage Diff              @@
##            master    #18774       +/-   ##
=============================================
- Coverage   75.661%   75.651%   -0.010%     
=============================================
  Files          647       647               
  Lines       168420    168420               
  Branches     39681     39682        +1     
=============================================
- Hits        127429    127413       -16     
- Misses       35442     35454       +12     
- Partials      5549      5553        +4     

@sylee957
Copy link
Member

sylee957 commented Mar 4, 2020

Can you also add some tests?

@sbt4104
Copy link
Contributor Author

sbt4104 commented Mar 4, 2020

I am adding it .

@sbt4104
Copy link
Contributor Author

sbt4104 commented Mar 4, 2020

Lambdify only takes numpy array as an input, it discards regular python array.
import numpy is giving me error on Travis CI

@sylee957
Copy link
Member

sylee957 commented Mar 4, 2020

The reason that your tests are failing is because you have not made that as an optional dependency test.
You should use the import_module logic to skip the test depending whether numpy is intalled or not.
I'd better keep the string output test in test_pycode, because it can be tested outside of the environment setup.

@asmeurer
Copy link
Member

asmeurer commented Mar 4, 2020

Look at how the other lambdify tests are written, so that they skip the test if numpy is not available.

@asmeurer
Copy link
Member

asmeurer commented Mar 4, 2020

I fixed the release notes entry. Please keep them so they are relevant to end-users, not referencing implementation details.

@sbt4104
Copy link
Contributor Author

sbt4104 commented Mar 4, 2020

I will keep that in mind

@sbt4104 sbt4104 requested a review from sylee957 March 5, 2020 10:18
@sylee957 sylee957 merged commit 9fa2d4a into sympy:master Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lambdify giving a different response pre- and post-rewrite
5 participants