blab / pathogen-embed

Create reduced dimension embeddings for pathogen sequences
https://pypi.org/project/pathogen-embed/
MIT License
1 stars 0 forks source link

Relax scikit-learn constraint to support all of major version 1 #26

Open corneliusroemer opened 1 month ago

corneliusroemer commented 1 month ago

I noticed you constrain scikit-learn to sub-major version. Is there a pressing reason for this? If not, would be good to relax as any environment with pathogen-embed (e.g. nextstrain's conda-base) is otherwise constrained by pathogen-embed.

scikit-learn latest version is 1.5.1, whereas you pin <1.5

huddlej commented 1 month ago

@corneliusroemer I haven't tested pathogen-embed with sklearn 1.5.* yet, since it was just released in May and I've been focused on revisions for the associated paper. The main reason I pinned <1.5 is that the scikit-learn release policy allows breaking changes on "minor" versions, according to the v1.0.0 release highlights:

This release does not include any breaking changes apart from the usual two-release deprecation cycle. For the future, we do our best to keep this pattern.

For example, a t-SNE keyword has a planned deprecation in version 1.7.

Given that release policy, I would rather opt into new versions of scikit-learn than allow them by default.

When I added pathogen-embed to the Nextstrain environments, that was the first time scikit-learn had been added as a Nextstrain dependency. If we find the pathogen-embed pinning restricts other future work, we could revisit my thinking above. Do you have a specific need for v1.5.* now?