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

Assert Statements are for Debugging #242

Closed
ssolson opened this issue May 19, 2023 · 3 comments · Fixed by #276
Closed

Assert Statements are for Debugging #242

ssolson opened this issue May 19, 2023 · 3 comments · Fixed by #276
Assignees
Labels
Clean Up Improve code consistency and readability

Comments

@ssolson
Copy link
Contributor

ssolson commented May 19, 2023

Currently MHKiT uses assert staements in its production code (primarilty for type checking functional inputs). As stated in the docs (https://docs.python.org/3/reference/simple_stmts.html#the-assert-statement) assert stements are for debugging.

We should raise an error if the user does not pass the correct information.

So going forward we would convert:

assert isinstance(station_number, (str, type(None))), (f'station_number must be  of type string')

to this

if not isinstance(station_number, (str, type(None))):
    raise ValueError(f'station_number must be of type string. Got: {station_number}')
@ssolson ssolson added the Clean Up Improve code consistency and readability label May 19, 2023
@ssolson ssolson self-assigned this May 19, 2023
@ssolson
Copy link
Contributor Author

ssolson commented May 19, 2023

Here is an article covering this idea in more detail:
https://snyk.io/blog/the-dangers-of-assert-in-python/

image

@ssolson ssolson mentioned this issue Jun 16, 2023
8 tasks
@ssolson ssolson mentioned this issue Jul 14, 2023
4 tasks
@onedeeper
Copy link

Hello! I'm trying to get my feet wet with some python open source. This seems straight forward so I'm happy to take care of it. Does this seem like a good first issue? Thanks!

@ssolson
Copy link
Contributor Author

ssolson commented Aug 10, 2023

Hello! I'm trying to get my feet wet with some python open source. This seems straight forward so I'm happy to take care of it. Does this seem like a good first issue? Thanks!

@onedeeper I am so sorry I missed this. Yes this would be a great first issue. Please lmk if you need any help contributing!

@akeeste akeeste self-assigned this Nov 6, 2023
@akeeste akeeste linked a pull request Nov 7, 2023 that will close this issue
10 tasks
akeeste added a commit that referenced this issue Nov 16, 2023
* loads/extreme convert asserts to errors

* loads/general convert asserts to errors

* loads/graphics convert asserts to errors

* power module convert asserts to errors

* loads module, convert try-except validation to errors

* utils module, convert asserts to errors

* dolfyn module, convert asserts to errors

* tidal module, convert asserts to errors

* river module, convert asserts to errors

* wave hindcast, replace asserts with errors

* wave/io module, convert asserts to errors

* wave module, convert asserts to errors

* rename reserved variable min in plot_directional_spectrum

* fix miscellaneous typos

* catch new error type

* fix test logic and parameter name

* minor review comments, fix f strings, fix messages, etc

* list correct types in error messages, f strings, etc

* standardize error messages for optional parameters

* Apply suggestions from ssolson's 2nd review
@ssolson ssolson mentioned this issue Apr 24, 2024
@akeeste akeeste closed this as completed Jul 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Up Improve code consistency and readability
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants