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

change parsing of the input in einsum #579

Merged
merged 6 commits into from Feb 27, 2023
Merged

change parsing of the input in einsum #579

merged 6 commits into from Feb 27, 2023

Conversation

HadrienNU
Copy link
Contributor

After testing the implemented einsum (many thanks for implementing), it turns out that the ellipsis were not working (and they are used by xarray dot function, my primary motivation for making this work). So in this commit there is an added function _parse_einsum input that is borrowed from numpy with the only change that operand are not cast to numpy array but are left untouched.

sparse/_common.py Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Feb 24, 2023

Codecov Report

Merging #579 (07d4932) into master (cb02d11) will decrease coverage by 0.04%.
The diff coverage is 91.66%.

@@            Coverage Diff             @@
##           master     #579      +/-   ##
==========================================
- Coverage   92.19%   92.16%   -0.04%     
==========================================
  Files          20       20              
  Lines        3204     3304     +100     
==========================================
+ Hits         2954     3045      +91     
- Misses        250      259       +9     

@hameerabbasi
Copy link
Collaborator

Seems like a formatting fail -- Perhaps update black to the latest version and re-run black .?

Also, there are a lot of lines missing from coverage, more comprehensive tests would be appreciated.

@HadrienNU
Copy link
Contributor Author

Ok, I will add more tests

Copy link
Collaborator

@hameerabbasi hameerabbasi left a comment

Choose a reason for hiding this comment

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

One last change!

sparse/tests/test_einsum.py Outdated Show resolved Hide resolved
@hameerabbasi hameerabbasi self-requested a review February 27, 2023 21:34
@hameerabbasi hameerabbasi merged commit 8b714d6 into pydata:master Feb 27, 2023
@hameerabbasi
Copy link
Collaborator

Thanks, @HadrienNU for the improvement!

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.

None yet

2 participants