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

Short circuit if query yields no results #82

Merged
merged 3 commits into from
Apr 1, 2022

Conversation

hancush
Copy link
Contributor

@hancush hancush commented Mar 23, 2022

Thanks so much for this awesome library!

This PR adds a logic to short circuit further queries if the initial result count is 0. It also adds a test for this behavior.

I made this change because the min/max IDs of an empty result set are null (e.g.), which causes _get_layer_min_max to fail with the error reported here: datamade/census_area#12

I thought it made more sense to stop if we know there are no results to return, rather than to handle the error in the min/max comparison.

I like that this approach is agnostic about whether no results is good or bad (as opposed to raising an error), but I'm happy for your feedback, if you'd prefer a different behavior!

Let me know what you think. 🙃

@iandees
Copy link
Member

iandees commented Apr 1, 2022

Thanks for this improvement! It looks good. Thanks especially for adding a test for it.

@iandees iandees merged commit 7aef868 into openaddresses:master Apr 1, 2022
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