MDAnalysis / UserGuide

User Guide for MDAnalysis
https://userguide.mdanalysis.org
22 stars 33 forks source link

ci: exercise both pip and conda tests #290

Closed jandom closed 1 year ago

jandom commented 1 year ago

exploratory PR, just checking if we can actually build from requirements.txt – i'm guessing we can't, i'm guessing we can only build from conda environment.yml and requirements.txt is stale

goal if we're truly supporting both conda and pip, we should demonstrate that both "legs" work – currently only the conda is exercised in CI/CD. New contributors arriving have a 50/50 chance of trying pip and conda, it's a big meh if one of them is broken.

update it seems like with enough massaging, requirements.txt can be made to build. Long-term two build system are just more confusing and harder to maintain. I know people care about their conda environments, but with pyproject.toml the community is moving in a different direction (pip/venv). Having a pyproject.toml would give us clear separation between test and package deps, as well.

jandom commented 1 year ago

I don't think it'll be possible to run tests with just a venv+pip – the tests require hole2 (only available via conda) and openmm (only available via conda). Thinking about the future: I'd put these "non-negotiable" conda deps into environment.yml, and everything else into the pyproject.toml – because it'll give us ability to do sub-packages

jandom commented 1 year ago

@IAlibay would be interesting to hear your comments on my mis-adventures in this PR (now closed)

lilyminium commented 1 year ago

@jandom I think as a whole the MDAnalysis does strongly encourage conda over pip, and not just because some dependencies are only on conda. From my corner of MD-related python I also see many more packages preferring conda over pip/venv, although I do wholly support modernising to pyproject.toml. We definitely shouldn't have stale requirements in requirements.txt but I do wonder if the best intermediate solution isn't just to remove that for now and eventually move to pyproject.toml.

jandom commented 1 year ago

Hi there @lilyminium I actually agree – I don't think than requirements.txt adds any value right now, it's semi-broken/broken.

What's the ideal end state?

Those are the two extremes I can see, both have pros and cons

IAlibay commented 1 year ago

What are you actually installing with your pyproject.toml?

If you're not creating an installable package out of the userguide my 2 cents is just stick to a conda only environment and the one yaml file. There's absolutely no need to ensure that pip only builds are supported, in fact it's imho an overcomplication that distracts from more important tasks.

IAlibay commented 1 year ago

To add here - if the context is to have extra testing for pypi installs, my other 2 cents is that we already test all the necessary optional dependencies upstream in the core library. At worst just do a pip install of MDAnalysis with all its optional deps rather than duplicating things here?

jandom commented 1 year ago

for pyproject.toml i'd be all the config and deps (build deps != deps for running tests). Yeah, I'd go for a full python package route (no publication tho). The single YAML file is partially compatible, it's very simple, everything in one place. But pyproject.tomml does support subpackages (test deps vs run deps) and it does support tool config (we could put mypy and other configs inside the pyproject.toml).

I agreed on the over complication, and at this stage can we at agree on removing requirements.txt? It's broken, it's confusing (it suggests that pip-only workflow is supported), doesn't add any value.

IAlibay commented 1 year ago

Yeah I'm ok with removing requirements.txt for now.