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

Add check if radars tuple is not empty #991

Merged
merged 2 commits into from Apr 29, 2021
Merged

Add check if radars tuple is not empty #991

merged 2 commits into from Apr 29, 2021

Conversation

Quba1
Copy link
Contributor

@Quba1 Quba1 commented Apr 29, 2021

This is the same PR as #990 (reopened as I have forgotten to add the check to the map_to_grid function).

In some edge cases (in my case when radars tuple is created in a loop) the empty tuple can be passed to the gates_to _grid or map_to_grid functions. In that casetuple index out of range error occures when radars[0].altitude['data'] is acessed, and it is not always obvious what is the cause of that error.

I propose to add a simple check if radars tuple length is equal to 0. If yes then ValueError exception is raised with a more meaningful error message indicating what the issue is.
The placement of that if statement is just after conversion of single Radar object into radars tuple, so that tuple length can be checked without worring about edge cases.
In this solution also length of gatefilters tuple is implicitly checked as gatefilters must have the same length as radars.
The check is also added to grid_from_radars function as there is no need to call the gridding functions if the radars tuple is empty.

@zssherman
Copy link
Collaborator

zssherman commented Apr 29, 2021

Thanks @Quba1! Everything looks good, down the road we may need a new unit tests to pytest raise the error. Merging.

@zssherman zssherman merged commit ea9450a into ARM-DOE:master Apr 29, 2021
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.

None yet

2 participants