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

Clarify error messages #232

Merged
merged 1 commit into from
Mar 1, 2019

Conversation

hameerabbasi
Copy link
Collaborator

Fixes #231
Fixes #230

@asmeurer Please make sure that these error messages are clear to you... I'm used to speaking deep NumPy-speak, and so I may be a bit blinded. 🙂

@codecov
Copy link

codecov bot commented Feb 21, 2019

Codecov Report

Merging #232 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #232   +/-   ##
=======================================
  Coverage   97.61%   97.61%           
=======================================
  Files          11       11           
  Lines        1549     1549           
=======================================
  Hits         1512     1512           
  Misses         37       37
Impacted Files Coverage Δ
sparse/coo/umath.py 97.13% <100%> (ø) ⬆️

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 bc81375...487cec0. Read the comment docs.

raise ValueError('Inconsistent fill-values in the result array: operating on the ndarray with'
' fill-values produces inconsistent results.')
raise ValueError('Performing a mixed sparse-dense operation that would result in a dense array. '
'Please make sure that func(sparse_fill_values, ndarrays) is a constant array.')
Copy link
Member

Choose a reason for hiding this comment

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

What is func?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The element-wise function one wants to apply.

@@ -507,8 +507,8 @@ def _check_broadcast(self):
)

if full_shape != non_ndarray_shape:
raise ValueError('All ndarrays must be broadcastable to the shape without ndarrays {}'
.format(non_ndarray_shape))
raise ValueError('Please make sure that the broadcast shape of just the sparse arrays is '
Copy link
Member

Choose a reason for hiding this comment

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

So in the case of a binary operation between dense and sparse as in the issue, "just the sparse arrays" is the one array, so broadcasting the sparse array is not allowed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Correct.

@hameerabbasi
Copy link
Collaborator Author

@asmeurer Do you think these error messages are fine or do they need refining? Feel free to suggest other (clearer) wordings. 🙂

@hameerabbasi hameerabbasi merged commit df984d3 into pydata:master Mar 1, 2019
@hameerabbasi hameerabbasi deleted the clearer-error-messages branch March 1, 2019 08:51
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