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

Fix parsing of $not expression on command line #970

Merged
merged 9 commits into from
Feb 2, 2024
Merged

Conversation

cbkerr
Copy link
Member

@cbkerr cbkerr commented Jan 25, 2024

Description

  • fixes bug
  • adds test to test_shell.py
  • I do not fix the testing code in test_find_command_line_interface.py that does not properly test the code path but I left a todo for now.

Motivation and Context

The $not expression was broken on the command line. I noticed this while working on glotzerlab/signac-docs#199

signac was inserting sp. prefixes before the operator

$ signac find '{"$not": {"a": 1}}'
Error: "Invalid operator placement 'sp.$not.a'."

The function filterparse._add_prefix does the inserting, which later triggered an error in _search_indexer._find_expression. This code path wasn't tested.

Checklist:

@cbkerr cbkerr added the bug Something isn't working label Jan 25, 2024
@cbkerr cbkerr requested review from a team as code owners January 25, 2024 21:24
@cbkerr cbkerr requested review from kidrahahjo and jennyfothergill and removed request for a team January 25, 2024 21:24
Copy link

codecov bot commented Jan 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f9df893) 85.65% compared to head (b4b0cc2) 85.71%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #970      +/-   ##
==========================================
+ Coverage   85.65%   85.71%   +0.06%     
==========================================
  Files          20       20              
  Lines        3464     3466       +2     
  Branches      759      760       +1     
==========================================
+ Hits         2967     2971       +4     
+ Misses        338      337       -1     
+ Partials      159      158       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

tests/test_shell.py Outdated Show resolved Hide resolved
signac/filterparse.py Show resolved Hide resolved
@cbkerr cbkerr requested a review from bdice January 26, 2024 19:41
Copy link
Member

@bdice bdice left a comment

Choose a reason for hiding this comment

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

I'd like to see test_shell avoid importing from test_find_command_line_interface, but I won't block this PR on that.

@cbkerr cbkerr enabled auto-merge (squash) February 2, 2024 21:58
@cbkerr cbkerr merged commit 124af48 into main Feb 2, 2024
16 checks passed
@cbkerr cbkerr deleted the fix/not-expression-cli branch February 2, 2024 22:04
@cbkerr cbkerr added this to the v2.2.0 milestone Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants