Skip to content

[MAINT] Updated pinned versions of dependencies in setup.cfg#12

Closed
chiuhoward wants to merge 14 commits intotractometry:mainfrom
chiuhoward:main
Closed

[MAINT] Updated pinned versions of dependencies in setup.cfg#12
chiuhoward wants to merge 14 commits intotractometry:mainfrom
chiuhoward:main

Conversation

@chiuhoward
Copy link
Contributor

@chiuhoward chiuhoward commented Dec 5, 2024

Limited manual test coverage in conda environment with python == 3.11. numpy2 breaks AFQ-Insight.
Closes #11.

Changes:

  • Updated pinned versions for scipy, dipy, matplotlib, numpy, pandas, seaborn, statsmodels, and scikit-learn.
  • Changed imports for deprecated sklearn functions in serial_bagging.
  • Change contributing.md references for repo URL from richford to tractometry

@chiuhoward
Copy link
Contributor Author

chiuhoward commented Dec 5, 2024

If I’m understanding the failing tests correctly, it has to do with base_estimator being deprecated since 1.4.0? Maybe I could pin it to 1.3.2 as a short-term fix until the refactoring comes in. I don’t know enough to know that we can just replace all instances of base_estimator with estimator, maybe the parameters it takes also change...

scikit-learn/scikit-learn#23819

@arokem
Copy link
Member

arokem commented Dec 5, 2024

@chiuhoward : I see a few different errors here:

I think that the three latter ones are essentially the same issue that you identified. But there are also the three others to contend with. I am going to try to fetch your branch, and see whether I can diagnose some of these.

For future reference, it's easier to work with PRs (and will also make your life easier) if you make the PR from a separate branch, and not from your main branch.

@chiuhoward
Copy link
Contributor Author

chiuhoward commented Dec 8, 2024

I've tried reading the documentation:

  1. I think OneHotEncoder needs sparse_output instead of sparse, but from the test log it seems like it's being imported by neurocombat rather than AFQ-Insight? I can't find any mention of OneHotEncoder in the AFQ-Insight repo (https://scikit-learn.org/stable/modules/generated/sklearn.preprocessing.OneHotEncoder.html)

  2. I think def _fit_and_score_ckpt is where we introduce score_params as an additional kwarg, but I don't know how/where to concretely implement this. https://github.com/scikit-learn/scikit-learn/blob/6e9039160f0dfc3153643143af4cfdca941d2045/sklearn/model_selection/_validation.py#L740

@arokem
Copy link
Member

arokem commented Dec 9, 2024

We do depend on neurocombat_sklearn, which has not been updated in a while...

I wonder whether this is something that the nilearn community may be interested in taking up and maintaining in the long run.

@arokem
Copy link
Member

arokem commented Dec 9, 2024

Maybe for now our best option is to vendorize neurocombat_sklearn into our code-base and fix it up here. It's MIT-licensed and less than 400 lines of code, with only sklearn and numpy dependencies, so shouldn't be too bad to bring over.

@chiuhoward
Copy link
Contributor Author

closed by #20

@chiuhoward chiuhoward closed this Feb 3, 2025
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.

[suggestion] upgrade sklearn

2 participants