tedana icon indicating copy to clipboard operation
tedana copied to clipboard

Change n_iter to max_iter in TSNE parameters

Open miltoncamacho opened this issue 2 weeks ago • 5 comments

Updates the argument for sklearn.manifold.TSNE from n_iter to max_iter.

Closes #1275

miltoncamacho avatar Dec 12 '25 23:12 miltoncamacho

Codecov Report

:x: Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 89.83%. Comparing base (ac50072) to head (e75d656). :warning: Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
tedana/decomposition/ica.py 71.42% 1 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1276      +/-   ##
==========================================
- Coverage   89.86%   89.83%   -0.04%     
==========================================
  Files          29       29              
  Lines        4383     4389       +6     
  Branches      725      726       +1     
==========================================
+ Hits         3939     3943       +4     
- Misses        295      296       +1     
- Partials      149      150       +1     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

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

codecov[bot] avatar Dec 12 '25 23:12 codecov[bot]

What is the new minimum version after this change? What version of sklearn made the change? We have to make sure that's reflected in pyproject.toml.

eurunuela avatar Dec 13 '25 01:12 eurunuela

The version of sklearn is 1.8.0, but the test fails because scikit-learn 1.8.0 requires python version >=3.11 and tedana only need >=3.9. Is this something we feel is doable?

  × No solution found when resolving dependencies for split (markers:
  │ python_full_version == '3.10.*'):
  ╰─▶ Because the requested Python version (>=3.9) does not satisfy
      Python>=3.11 and scikit-learn==1.8.0 depends on Python>=3.11, we can
      conclude that scikit-learn==1.8.0 cannot be used.
      And because only scikit-learn<=1.8.0 is available, we can conclude that
      scikit-learn>=1.8.0 cannot be used.
      And because your project depends on scikit-learn>=1.8.0 and your project
      requires tedana[all], we can conclude that your project's requirements

miltoncamacho avatar Dec 13 '25 17:12 miltoncamacho

fMRIPrep builds on 3.10 so we won't want to increase our minimum Python version above that. I don't know what the situation is with AFNI.

tsalo avatar Dec 13 '25 18:12 tsalo

For now, I recommend checking the sklearn version and modifying the parameter name based on that.

tsalo avatar Dec 13 '25 18:12 tsalo

The changes look good to me. I just made a couple of suggestions to hopefully address the linter errors.

tsalo avatar Dec 17 '25 20:12 tsalo