Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make all mdakits imported & used in the core library optional and don't ship them on the conda-forge recipe with v2.9 #4899

Open
IAlibay opened this issue Jan 25, 2025 · 9 comments
Assignees
Labels

Comments

@IAlibay
Copy link
Member

IAlibay commented Jan 25, 2025

I want to do a py3.13 release that works on conda-forge, but the way we are currently dealing with deprecated analysis modules that are now mdakits is blocking me.

Long story short, there's a cyclic packaging issue that is resolvable on the medium term (make base packages etc..), but we don't have time to deal with this on the short term and I would love to unblock a lot of py3.13 compatibility issues downstream.

I realise this effectively means we break the API early but I don't think it's worth blocking a major python release for analyses that are deprecated and we can just say "install this package and things will work as you expect them for now".

@IAlibay
Copy link
Member Author

IAlibay commented Jan 25, 2025

Could I get the @MDAnalysis/coredevs to weigh in on a one week timeline please?

Ideally I would like to have a decision made by next weekend so we can move ahead with a new release.

@orbeckst orbeckst added release decision needed requires input from developers before moving further labels Jan 25, 2025
@orbeckst
Copy link
Member

I am ok with the proposed solution. Py 3.13 is important.

Given that we're sort-of breaking semantic versioning (at least at the installation level, not at the code level) we should

  • be clear in our CHANGELOG and release blog post about what's happening
  • have clear error messages when the optional import fails

@orbeckst orbeckst reopened this Jan 25, 2025
@orbeckst
Copy link
Member

orbeckst commented Jan 25, 2025

(sorry didn't intend to close the issue, pressed wrong button)

Quick question: How will this work with the PyPi install?

@IAlibay
Copy link
Member Author

IAlibay commented Jan 25, 2025

(sorry didn't intend to close the issue, pressed wrong button)

Quick question: How will this work with the PyPi install?

Same thing as conda-forge unfortunately - folks will need to manually install the additional kits.

We can make the warning messages very clear about how they can go about doing so though (i.e. put the pip & conda install options in the warning).

@RMeli
Copy link
Member

RMeli commented Jan 26, 2025

I think with a clear warning, this should only be a minor annoyance to the users. Or at least I hope so. I'd be in favour of this change to move things forward.

@yuxuanzhuang
Copy link
Contributor

I am also in favor of this change.

@tylerjereddy
Copy link
Member

Sounds "ok," especially if each such import has already been deprecated. In which release did the deprecation happen? I think there's a convention in the community to "execute" deprecations after warning for two releases. If you're cutting to just 1 release where you warned that might be a bit tighter than desired, but for getting 3.13 support ready on limited bandwidth I'd agree with the others that it seems worth it even if not ideal.

@IAlibay
Copy link
Member Author

IAlibay commented Jan 31, 2025

In which release did the deprecation happen?

I think at least one was in the last release unfortunately.

@IAlibay
Copy link
Member Author

IAlibay commented Feb 3, 2025

Thanks all, it's been over a week so I'm going to assume that this is now approved. I'll go ahead with this change next weekend.

@IAlibay IAlibay self-assigned this Feb 3, 2025
@orbeckst orbeckst removed the decision needed requires input from developers before moving further label Feb 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants