Skip to content

BUG: Raise WMQL errors instead of Python exceptions for invalid queries - #133

Open
demianw wants to merge 1 commit into
masterfrom
fix-invalid-query-errors
Open

demianw wants to merge 1 commit into
masterfrom
fix-invalid-query-errors

Conversation

@demianw

@demianw demianw commented Nov 4, 2026 •

Copy link
Copy Markdown
Owner

What was wrong

Each row of the table in #103 reproduces on master. I ran queries_preprocess + eval_queries against a small spatial index with non-empty label bounding boxes. Two of the rows cannot be reached through queries_syntax_check, because it uses an empty atlas.

Query Master Now
a = 1 / b = a.x.y TypeError: not enough arguments for format string TractQuerierSyntaxError: Invalid query in line 2: a.x.y
a = 1 / for x in 3: / b = x UnboundLocalError: list_items TractQuerierSyntaxError: Error in FOR statement in line 2, it must iterate over a list of query names or a string pattern
import f.qry where f.qry has a syntax error TypeError: 'SyntaxError' object is not subscriptable TractQuerierSyntaxError: syntax error in file <path>/f.qry line N: <line>
a = 1 / b = medial_of(a) (also lateral_of) UnboundLocalError: side TractQuerierSyntaxError: medial_of needs a query ending in .left or .right. Line 2
a = 1 - 1 / b = anterior_of(a) (empty label set; also an unmatched 'pattern*') StopIteration TractQuerierLabelNotFound: anterior_of applied to a query with no labels. Line 2

I found one more crash in the same branch of process_relative_term, not listed in the issue: a.foo = 1 / b = anterior_of(a.foo) gave UnboundLocalError: query_info. It now raises the existing Attribute not recognized for relative specification error.

Change

All changes are in tract_querier/query_processor.py:

  • visit_Attribute: give the format string both of its arguments. The message shows ast.unparse(node).
  • visit_For: add an else: raise for iterables that are neither a list nor a string.
  • RewritePreprocess.visit_Import: build the message from the SyntaxError (filename, lineno, text). Before, it used Python 2 tuple indexing, and module_name was an ast.alias.
  • process_relative_term: side now starts as None, and only Name or .left/.right arguments are accepted. medial_of/lateral_of without a side raise an error. This validation now runs before the empty-atlas return FiberQueryInfo(), so queries_syntax_check (which the CLI runs before loading data) also catches these errors. An empty label set raises TractQuerierLabelNotFound.

Behaviour change / maintainer decision: a relative term over a query with an empty label set now raises TractQuerierLabelNotFound instead of StopIteration. The alternative is to return an empty result, as the function already does when the atlas has no labels at all. None of the packaged .qry files can produce an empty label set: their relative-term arguments are all label-defined *.side queries. So raising does not affect them. If you prefer the empty result, it is a one-line change.

Valid queries return the same results as before.

Testing

  • New tract_querier/tests/test_query_errors.py, with a parametrised test of every row above plus the a.foo case, checking the exception class and message. It also includes a queries_syntax_check test for the rows reachable there, and a positive medial_of/anterior_of test. On master's query_processor.py, 11 of these 12 tests fail. The positive test passes on both.
  • python -m pytest -q -p no:cacheprovider tract_querier: 54 passed, 1 skipped. This includes test_query_files, which checks the syntax of every packaged query file.

Merge order: independent of #130 (#38), which touches queries_preprocess a few lines below visit_Import. The two may conflict textually. They use the same new syntax error in file ... line ... wording. #139 (#115, Python 2 leftovers) touches visit_Call in the same file, but in different lines.

Closes #103

🤖 Generated with Claude Code

Several error paths in the query evaluator crashed with Python errors:

- `b = a.x.y`: the error message had one `%` argument for two `%s`
  (TypeError). It now names the expression.
- `for x in 3:` left `list_items` unbound (UnboundLocalError). Iterating
  over anything but a list of names or a string pattern now raises.
- `import f` with a syntax error in `f` used Python 2 `exc_value[1][1]`
  (TypeError) and named an `ast.alias` object as the file. It now reports
  the file name, line and text from the SyntaxError.
- `medial_of(a)` / `lateral_of(a)` without `.left`/`.right` left `side`
  unbound, and `anterior_of(a.foo)` left `query_info` unbound. The argument
  is now validated, before the empty-atlas shortcut so that
  `queries_syntax_check` also reports it.
- A relative term over a query with no labels leaked `StopIteration`; it
  now raises TractQuerierLabelNotFound.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

BUG: invalid queries crash with Python exceptions instead of WMQL errors

1 participant