demianw / tract_querier

Implementation of the White Matter Query Language and associated tools for dMRI white matter tract extraction and analysis
Other
26 stars 27 forks source link

More Python 3 updates, fix #41 #45

Closed ihnorton closed 5 years ago

ihnorton commented 5 years ago

Some more python 3 fixes, starting from #41. Fixes some ast usage issues (to fix #44), and various other errors we encountered running locally under python 3.

Also fixes several remaining nosetest failures, except this one below (I'll fix it later on):

======================================================================
FAIL: tract_querier.tests.test_query_rewrite.test_rewrite_notin_precedence
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/opt/sw/miniconda36/envs/tract_querier/lib/python3.6/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/Users/inorton/work/pnl/git/tract_querier/tract_querier/tests/test_query_rewrite.py", line 55, in test_rewrite_notin_precedence
    assert_equal(ast.dump(tree3), ast.dump(tree3_rw))
AssertionError: "Modu[15 chars]alue=BoolOp(op=And(), values=[Name(id='a', ctx[104 chars]))])" != "Modu[15 chars]alue=Compare(left=BoolOp(op=And(), values=[Nam[104 chars]))])"
Diff is 643 characters long. Set self.maxDiff to None to see it.

----------------------------------------------------------------------

cc @tashrifbillah

tashrifbillah commented 5 years ago

Isaiah's fix successfully runs pnlpipe. So, we shall update our software against his pull request.

tashrifbillah commented 5 years ago

cc @demianw

demianw commented 5 years ago

@tashrifbillah @ihnorton great work! I will have time from wednesday to see that we can merge this PR ASAP.

tashrifbillah commented 5 years ago

Given a .vtk file and an fsInDWI, I have also checked the following equivalence between python2 and python 3 outputs. They agree.

    Query af.right: 000225
    Query cb.left: 000000
    Query cb.right: 000000
    Query cc: 000014
    Query cc_1: 000000
    Query cc_2: 000000
    Query cc_3: 000002
    Query cc_4: 000000
    Query cc_5: 000001
    Query cc_6: 000009
    Query cc_7: 000002
    Query cc_bankssts: 000000
    Query cc_caudalanteriorcingulate: 000000
    Query cc_caudalmiddlefrontal: 000411
    Query cc_cuneus: 000000
    Query cc_entorhinal: 000000
    Query cc_frontalpole: 000000
    Query cc_fusiform: 000000
    Query cc_inferiorparietal: 000000
    Query cc_inferiortemporal: 000000
    Query cc_insula: 000000
    Query cc_isthmuscingulate: 000000
    Query cc_lateraloccipital: 000000
    Query cc_lateralorbitofrontal: 000014
    Query cc_lingual: 000000
    Query cc_medialorbitofrontal: 000000
    Query cc_middletemporal: 000000
    Query cc_paracentral: 000000
    Query cc_parahippocampal: 000000
    Query cc_parsopercularis: 000002
    Query cc_parsorbitalis: 000000
    Query cc_parstriangularis: 000000
    Query cc_pericalcarine: 000000
    Query cc_postcentral: 000000
    Query cc_posteriorcingulate: 000000
    Query cc_precentral: 000000
    Query cc_precuneus: 000000
    Query cc_rostralanteriorcingulate: 000000
    Query cc_rostralmiddlefrontal: 003486
    Query cc_superiorfrontal: 001309
    Query cc_superiorparietal: 000000
    Query cc_superiortemporal: 000018
    Query cc_supramarginal: 000002
    Query cc_temporalpole: 000000
    Query cc_transversetemporal: 000002
    Query ccommisural: 005056
    Query cst.left: 000389
    Query cst.right: 000203
    Query ec.left: 000277
    Query ec.right: 000064
    Query extreme_capsule.left: 000114
    Query extreme_capsule.right: 000055
    Query ilf.left: 000000
    Query ilf.right: 000000
    Query internal_capsule_new.left: 000621
    Query internal_capsule_new.right: 000315
    Query ioff.left: 000037
    Query ioff.right: 000024
    Query mdlf.left: 000000
    Query mdlf.right: 000010
    Query slf_i.left: 000098
    Query slf_i.right: 000656
    Query slf_ii.left: 000147
    Query slf_ii.right: 000025
    Query slf_iii.left: 000013
    Query slf_iii.right: 000034
    Query soff.left: 000049
    Query soff.right: 000003
    Query striato_frontal.left: 000166
    Query striato_frontal.right: 000254
    Query striato_occipital.left: 000000
    Query striato_occipital.right: 000000
    Query striato_parietal.left: 000172
    Query striato_parietal.right: 000060
    Query thalamo_frontal.left: 000144
    Query thalamo_frontal.right: 000218
    Query thalamo_occipital.left: 000008
    Query thalamo_occipital.right: 000006
    Query thalamo_parietal.left: 000145
    Query thalamo_parietal.right: 000083
    Query uf.left: 000007
    Query uf.right: 000000
tashrifbillah commented 5 years ago

@demianw , how is going with the PR?

codecov[bot] commented 5 years ago

Codecov Report

Merging #45 into master will decrease coverage by 0.59%. The diff coverage is 67.56%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #45     +/-   ##
=========================================
- Coverage   45.06%   44.47%   -0.6%     
=========================================
  Files          27       26      -1     
  Lines        2543     2559     +16     
=========================================
- Hits         1146     1138      -8     
- Misses       1397     1421     +24
Impacted Files Coverage Δ
tract_querier/tract_math/tract_operations.py 22.5% <0%> (+0.54%) :arrow_up:
tract_querier/tract_math/tensor_operations.py 8.25% <0%> (ø) :arrow_up:
tract_querier/shell.py 19.4% <0%> (ø) :arrow_up:
tract_querier/aabb.py 25% <0%> (ø) :arrow_up:
tract_querier/tractography/trackvis.py 83.01% <100%> (+0.66%) :arrow_up:
tract_querier/tensor/__init__.py 100% <100%> (ø) :arrow_up:
tract_querier/tests/__init__.py 100% <100%> (ø) :arrow_up:
tract_querier/tract_math/__init__.py 100% <100%> (ø) :arrow_up:
tract_querier/tensor/tests/test_scalar_measures.py 100% <100%> (ø) :arrow_up:
tract_querier/tests/test_scripts.py 35.55% <100%> (+1.46%) :arrow_up:
... and 14 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 f956c09...814efcd. Read the comment docs.

tashrifbillah commented 5 years ago

Cool, now I guess we should be good to merge this PR!

demianw commented 5 years ago

Finally found the time to complete this PR and merge it. Thanks for your work @tashrifbillah and @ihnorton !!!!