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

filter: Handle errors from filter_by_query #942

Merged
merged 2 commits into from
Jun 17, 2022

Conversation

victorlin
Copy link
Member

@victorlin victorlin commented May 20, 2022

Description of proposed changes

  • The underlying pandas engine used for --query can raise various errors depending on the input.
  • The query call is invoked by a loop through all filtering functions.

Based on the above points, this change wraps the line executing filter functions and catches all errors, but only handles the errors from filter_by_query.

For the errors from filter_by_query, the UndefinedVariableError is predictable enough to mean that a column does not exist. For all other errors, raise a generic error message directing users to the pandas query syntax doc page.

Related issue(s)

Testing

Added functional tests.

@victorlin victorlin self-assigned this May 20, 2022
@codecov
Copy link

codecov bot commented May 20, 2022

Codecov Report

Merging #942 (bc670ba) into master (5916362) will increase coverage by 0.00%.
The diff coverage is 85.71%.

@@           Coverage Diff            @@
##           master     #942    +/-   ##
========================================
  Coverage   59.34%   59.34%            
========================================
  Files          42       50     +8     
  Lines        6011     6257   +246     
  Branches     1539     1584    +45     
========================================
+ Hits         3567     3713   +146     
- Misses       2185     2283    +98     
- Partials      259      261     +2     
Impacted Files Coverage Δ
augur/filter.py 96.10% <85.71%> (-0.13%) ⬇️
augur/util_support/node_data_file.py 100.00% <0.00%> (ø)
augur/io_support/shell_command_runner.py
augur/io.py
augur/io/file.py 100.00% <0.00%> (ø)
augur/util_support/metadata_file.py 100.00% <0.00%> (ø)
augur/io/shell_command_runner.py 91.89% <0.00%> (ø)
augur/io/sequences.py 100.00% <0.00%> (ø)
augur/io/__init__.py 100.00% <0.00%> (ø)
augur/measurements.py 31.25% <0.00%> (ø)
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5916362...bc670ba. Read the comment docs.

@victorlin victorlin force-pushed the filter/handle-query-errors branch 2 times, most recently from f339f40 to 62c21fc Compare May 24, 2022 21:37
@victorlin victorlin marked this pull request as ready for review May 24, 2022 21:37
@victorlin victorlin requested a review from a team May 24, 2022 21:38
Copy link
Member

@tsibley tsibley left a comment

Choose a reason for hiding this comment

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

Great to do this sort of nicer error handling! A few minor notes; nothing truly blocking if you object.

- The underlying pandas engine used for --query can raise various errors depending on the input.
- The query call is invoked by a loop through all filtering functions.

Based on the above points, this change wraps the line executing filter functions and catches all errors, but only handles the errors from filter_by_query.

For the errors from filter_by_query, the UndefinedVariableError is predictable enough to mean that a column does not exist. For all other errors, raise a generic error message directing users to the pandas query syntax doc page.
@victorlin victorlin force-pushed the filter/handle-query-errors branch from 62c21fc to d1a063a Compare June 17, 2022 22:30
@victorlin
Copy link
Member Author

@tsibley thanks for the pointers! Will merge when checks pass.

@victorlin victorlin merged commit bf400de into nextstrain:master Jun 17, 2022
@victorlin victorlin deleted the filter/handle-query-errors branch June 17, 2022 23:04
@victorlin victorlin added this to the Next release X.X.X milestone Jun 21, 2022
@victorlin victorlin mentioned this pull request Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
2 participants