-
Notifications
You must be signed in to change notification settings - Fork 8
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
include dtype in np.random.uniform addressing #175 #176
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.
LGTM @delgadom but can you also add a changelog entry documenting the bug fix?
Thanks for the quick turn-around @delgadom. Agreed with Diana, but just add this PR number and @ your username at the end of the
line in CHANGELOG. We don't need a new entry because Emile's change hasn't been released. I want to check to see if this still works in one other weird case we had with regridding... I'll follow up. |
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.
Thanks @delgadom! Appreciate the turnaround on this.
My only suggestion is like what Diana says, please update the CHANGELOG to append your name and this PR number onto Emile's entry for PR #174. Like (PR #174, PR #176, @emileten, @delgadom )
. I don't feel like this needs an entirely new CHANGELOG entry.
Aside from that, I think this is ready to merge when you're ready.
I'll update the changelog as reviews suggested and merge. Thanks for the quick help everyone! |
In this PR: