Closed akeeste closed 7 months ago
@akeeste just checking if this is ready for another review
@akeeste just checking if this is ready for another review
@ssolson Yes. I implemented all of the suggested changes. Let me know if there's any other changes you'd like to see.
I'm debating if the update to the min
parameter of plot_directional_spectrum
is necessary. How much of an issue is it to overwrite the python function min
? Seems safer to not overwrite it, but the function call won't be backwards compatible then.
I'm debating if the update to the
min
parameter ofplot_directional_spectrum
is necessary. How much of an issue is it to overwrite the python functionmin
? Seems safer to not overwrite it, but the function call won't be backwards compatible then.
Its a very easy fix for the user to change a kwarg from min
to color_bar_min
and is the correct way to handle.
Thanks @ssolson. I resolved those last few issues and will merge
This PR replaces all non-testing asserts with code that will raise errors. For the vast majority of asserts, the error messages are unchanged or have additions to say the parameter type or value that was given and is causing an issue.
Some try-except statements appeared like they should be errors, but just defined unused strings. I updated these to throw error messages.
Modules completed:
I left
river/io/d3d.py
alone for now because #271 edits it and I wanted to avoid conflicts if possible.Other items
plot_directional_spectrum
. It was using a parametermin
that overwrote the built in Python function. i changed the parameter tocolor_level_min
to prevent potential issues.