-
Notifications
You must be signed in to change notification settings - Fork 178
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
Mean 1d #130
Mean 1d #130
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These improvements look really good to me (I tried to achieve a similar thing but without success; the reduce_shape is really useful). Thanks @aradi .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please submit the formatting changes in a separate PR? Let's not pollute our PRs with formatting.
Btw, the style guide says to use 4 spaces:
Either way, that is irrelevant to the actual changes in this PR, which all look good to me. So if you could please split the formatting into a new PR, then let's get it PR merged.
Well, the styleguide says Anyway, I have taken out the reformatting commit, it really does not belong to here, I agree. I'll wait with reformatting changes until the style guide becomes non-ambigous. 😉 |
@aradi the PR looks great, thank you! |
mean()
.mean()
to handle reduction from 1D-array to scalar (similar tosum()
)== 0.0
is highly unreliable and may give false positives)Commits in order of relevance, feel free to cherry pick if you don't like all of them.