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

Improve error handling of dataset names in region_series #56

Merged
merged 4 commits into from
Sep 20, 2024

Conversation

japilo
Copy link

@japilo japilo commented Sep 18, 2024

I was running into some issues with names of subdatasets for region_series, so I implemented a couple of changes to improve it.

Issue 1: Uninformative error message in location_slice_from_region_series

I used this function and it threw a very uninformative error message. I tracked down the source of the error, and it was because the SpatRasterDataset I entered for the region_series argument did not have proper names for the subdatasets. I have added an informative error message that informs users if the names of region_series are invalid.

Issue 2: Uninformative error messages in slice_region_series

I also got an uninformative error message in the case when the subdatasets of region_series have improper varnames. I have added an informative error message that informs users if the varnames of region_series are invalid.

dramanica and others added 3 commits August 5, 2024 11:48
New issue template
I was running into some issues with names of subdatasets for region_series, so I implemented a couple of changes to improve it.
@dramanica
Copy link
Member

Thank @japilo! Would you be able to add a unit test to show when this check comes into play? Thank you for your help with the package!

@japilo
Copy link
Author

japilo commented Sep 19, 2024

I have added unit tests that demonstrate situations where I faced uninformative error messages previously, and now get informative error messages. I also added tests for the error handling that was already built into the function because I noticed they didn't have test coverage yet.

@dramanica dramanica changed the base branch from master to dev September 20, 2024 06:29
@dramanica
Copy link
Member

Brilliant, thank you @japilo!!! (only one minor request, in the future, can you please work off dev rather than master?)

@dramanica dramanica self-requested a review September 20, 2024 07:17
Copy link
Member

@dramanica dramanica left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you!

@dramanica dramanica merged commit 61e8499 into EvolEcolGroup:dev Sep 20, 2024
5 checks passed
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.

2 participants