ENH: Prefer using visit_Constant

[jhlegarreta]

Prefer using visit_Constant instead of visit_Num and visit_Str: both were deprecated since Python 3.8.

Fixes:

tract_querier/tests/test_query_eval.py: 40 warnings
tract_querier/tests/test_query_files.py: 6527 warnings
  /opt/hostedtoolcache/Python/3.9.20/x64/lib/python3.9/ast.py:407:
 DeprecationWarning: visit_Num is deprecated; add visit_Constant
    return visitor(node)

and

tract_querier/tests/test_query_eval.py: 1 warning
tract_querier/tests/test_query_files.py: 24 warnings
  /opt/hostedtoolcache/Python/3.9.20/x64/lib/python3.9/ast.py:407:
 DeprecationWarning: visit_Str is deprecated; add visit_Constant
    return visitor(node)

raised for example in:
https://github.com/demianw/tract_querier/actions/runs/12187495803/job/33998410948?pr=61#step:6:75 and
https://github.com/demianw/tract_querier/actions/runs/12187495803/job/33998410948?pr=61#step:6:80

Documentation:
https://docs.python.org/3/library/ast.html#ast.NodeVisitor.visit_Constant

[demianw]

I think this is better done by isinstance(node.value, (int, float, complex)) or isinstance(node.value, numbers.Number

[jhlegarreta]

Not sure: I had first tried with isinstance(node.value, (int, float, complex)) and the if was evaluated to True for a node.value that was a string, and thus the test was failing. I will revisit as time permits.

[jhlegarreta]

Changed to isinstance in e81a797. See the result of the debugging in the below comment.

[demianw]

this too is usually better as isinstance(node.s, str). Most probably this logic will need to be mixed with the one in line 555. Probably calling two methods, one for each type of constant

[jhlegarreta]

Not sure what's going on here: when debugging locally the change for EvaluateQueries seemed to work fine, but this one was either being evaluated to False for some cases or not being called at all (cannot remember) and thus the tests were failing.

[jhlegarreta]

If you happen to come up with a fix, it would be very much appreciated.

[jhlegarreta]

So I have digged into this a little bit more. I have used the following subset of labels (I named the file my_JHU_MNI_SS_WMPM_Type_II_subset.qry):

Fornix_column_and_body.left |= 51
Genu_of_corpus_callosum.left |= 52
Body_of_corpus_callosum.left |= 53
Splenium_of_corpus_callosum.left |= 54
Fornix_column_and_body.right |= 116
Genu_of_corpus_callosum.right |= 117
Body_of_corpus_callosum.right |= 118
Splenium_of_corpus_callosum.right |= 119

for the following subset of queries (named the file mori_queries_ssubset.qry):

import my_JHU_MNI_SS_WMPM_Type_II_subset.qry

HEMISPHERE.left |= '*.left'
HEMISPHERE.right |= '*.right'

corpus_callosum.side |= Genu_of_corpus_callosum.side or Body_of_corpus_callosum.side or Splenium_of_corpus_callosum.side

and using that file as the filename in test_query_files. It turns out that the method tract_querier.query_processor.RewritePreprocess.visit_Constant is being called when parsing the 51 part of Fornix_column_and_body.left |= 51, and since that is not a string instance, the new_node = self.visit(old_value) returned is None by the above visit_Constant block. Thus, the ast.NodeTransformer.generic_visit method will execute delattr(node, field), and the value field will get deleted. Further down the pipeline in tract_querier, at tract_querier.query_processor.EvaluateQueries.process_assignment we get an error as node.value does not exist:

E           AttributeError: 'AugAssign' object has no attribute 'value'

This does not happen in the previous implementation as tract_querier.query_processor.RewritePreprocess.visit_Str is not being called for the 51 part.

Following the above, and these:
https://github.com/python/cpython/blob/main/Lib/ast.py#L504
https://github.com/python/cpython/blame/9f9dac0a4e58d5c72aa3b644701cb155c009cb2c/Lib/ast.py#L415
following: python/cpython#81098 (comment)

adding

else:
    return self.generic_visit(node)

to tract_querier.query_processor.RewritePreprocess.visit_Constant
makes all the tests pass locally, including all test data query files.

And the CI shows that the related warnings are gone, e.g.:
https://github.com/demianw/tract_querier/actions/runs/12319259839/job/34385895407?pr=63#step:6:121
https://github.com/demianw/tract_querier/actions/runs/12319259839/job/34385896307?pr=63

The remaining warnings are related to NiBabel. x-ref PR #64.

@demianw Let me know if the solution here is satisfying enough or not. I'd like to test on in vivo human data with some results with previous versions to be completely sure that this does not change the current behavior. If you happen to have some examples, would be happy to try.