Skip to content

Revert "REF: Remove unnecessary viz module" - #120

Open
arokem wants to merge 1 commit into
masterfrom
revert-118-ref/remove-unnecessary-viz
Open

Revert "REF: Remove unnecessary viz module"#120
arokem wants to merge 1 commit into
masterfrom
revert-118-ref/remove-unnecessary-viz

Conversation

@arokem

@arokem arokem commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

Reverts #118

@jhlegarreta and @skoudoro : should we reconsider in light of the use of these visualizations in this CLI function? https://github.com/tee-ar-ex/trx-python/blob/master/trx/workflows.py#L276

@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 16.32653% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.15%. Comparing base (8ebbaa6) to head (bb69859).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
trx/viz.py 16.32% 41 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #120   +/-   ##
=======================================
  Coverage   62.15%   62.15%           
=======================================
  Files          13       13           
  Lines        2658     2658           
=======================================
  Hits         1652     1652           
  Misses       1006     1006           
Flag Coverage Δ
unittests 62.15% <16.32%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@skoudoro

skoudoro commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator

keep my position of +1 to remove, the module and the associated CLI. Not sure why the CI's did not catch it in first place.

@jhlegarreta

jhlegarreta commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

I still believe that it should not be distributed with the package, but happy to chat in case my viewpoint does not faithfully represent what the purpose of the package is. I believe that the visualization CLI should dwell in DIPY.

MY bad for not having checked uses of the function across the code. PR #124 to remove the leftovers I we believe the decisions was right.

@arokem

arokem commented Aug 7, 2026

Copy link
Copy Markdown
Collaborator Author

Could probably close this, based on outcome of the conversation on #119

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.

3 participants