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

[proposal] Switch from tox to Nox for running continuous integration environments #16412

Open
namurphy opened this issue May 7, 2024 · 11 comments

Comments

@namurphy
Copy link
Contributor

namurphy commented May 7, 2024

Astropy uses tox to run the environments for its continuous integration checks. While tox is in most ways a fantastic tool, its main drawback is that the configuration file (tox.ini) is an INI file. I personally find tox.ini files to be quite difficult to understand, modify, and maintain.

Nox is a project that is similar to tox, except that the configuration file is written in Python. 🐍 I'm in the process of switching PlasmaPy's tox environments to Nox sessions. I've been finding it significantly easier to set up and understand sessions in noxfile.py compared to tox.ini. I propose that Astropy switches its test runner from tox to Nox.

For some background, I highly recommend Hynek Schlawack's blog post on "Why I Like Nox", and trying out Nox for new or personal projects.

Example noxfile.py

import nox

@nox.session
def tests(session): 
    """Run tests with pytest."""
    session.install('pytest')
    session.run('pytest')

This file defines the tests session, which can be run with nox -s tests. (Vocabulary difference: a tox environment corresponds to a Nox session.)

Tradeoffs ⚖️

Here are some of the pros and cons of making the switch.

✅ Reasons to switch to Nox

  • The configuration file would be written in Python! 🎉🎂💖🎆🪩🥦🐍
  • Sessions would be more maintainable in the long term because the configuration file would be written in the language that Astropy contributors are most likely to understand. 🌌
  • Sessions could make use of Python functionality. 🐍
  • Sessions could be generated dynamically. 🌠
  • Astropy contributors would no longer need to understand the INI format, since the repository would contain no more .ini files. ✨
  • Nox is being actively developed.
  • Writing a Nox session tends to be quicker than writing a tox environment, in my experience. ⏱️
  • Switching to Nox would likely save developer time in the long run while improving developer experience. ⌛

❌ Reasons to keep using tox

  • Switching to Nox would require perhaps a week of someone's time. This would include converting tox environments to Nox sessions, updating GitHub Actions, revising the relevant sections of the development guide, and resolving any unexpected issues that may arise.
  • Switching to Nox would require having to fully understand the contents of tox.ini. 😐
  • tox is currently doing its job satisfactorily, with enough Astropy contributors able to maintain it. 👥
  • tox is an actively developed, well-established, and widely used project.
  • Switching to Nox could disrupt some user workflows.
  • Switching to Nox would lead to differences with packages whose architecture is inspired by Astropy's.

Suggested process for switching

I recommend taking an incremental process rather than doing a monolithic PR:

  • Convert a subset of existing tox environments into Nox sessions in one PR (e.g., just the docs environments).
  • Update GitHub workflows to use the Nox sessions in a follow-up PR
  • Repeat.

There is a tool called tox-to-nox that can help convert tox environments to Nox sessions, though I haven't tried it yet.

@namurphy
Copy link
Contributor Author

namurphy commented May 7, 2024

One moderately nonsensical follow-up point: I'd say that nox is more astrophysically relevant than tox because $\mathrm{NO}_x$ chemicals have been observed in the ISM! 🌌 Nitric oxide was even astromolecule of the month in July 2007.

@neutrinoceros
Copy link
Contributor

Sessions could make use of Python functionality.

Any ideas in particular ?

Sessions could be generated dynamically.

My understanding of tox is that is already allows defining sessions by composition, which could be argued is the only kind of dynamism that we need. I'm personally worried that introducing more dynamism would inflate the maintenance burden, not reduce it.

Switching to nox would lead to differences with packages whose architecture is inspired by Astropy's.

This seems like a very important point. I don't think we want to alienate the ecosystem that bloomed from astropy, and there's been a consistent effort to keep workflows as similar as possible between repos. Moving astropy away from tox would likely mean that we'd need to help coordinated and affiliated packages do the same, and it sounds like a sizable effort.

@astrofrog
Copy link
Member

I'm against this for a couple of reasons 👎

  • The main reason is that while being able to code the configuration in Python is more flexible, it is (in my view) too flexible and we will end up with a large bloated Python file. In the past, we switched from using setup.py to putting as much as possible in setup.cfg and then pyproject.toml because it is much nicer and more maintainable to define this in a declarative file with a very specific syntax. It limits feature creep and also makes it possible for the configuration to be machine-readable (for instance if we ever want to do any automated updates across the astropy ecosystem). I occasionally come across packages that still have large and complex setup.py files and it is a bit of a nightmare to navigate these - I fear the same would happen here.
  • The other reason is the point that has already be made that this will cause divergence from all other infrastructure, coordinated, and affiliated etc packages and it will be a large amount of work to update all these.

@neutrinoceros
Copy link
Contributor

it is (in my view) too flexible and we will end up with a large bloated Python file.

That's also what I meant in my first point, and I fully agree with your reasoning. Thanks for articulating it more clearly than I did 😄

@namurphy
Copy link
Contributor Author

namurphy commented May 8, 2024

Thank you for the replies — they are very helpful and insightful, and definitely aspects we need to consider!

Sessions could make use of Python functionality.

Any ideas in particular ?

For PlasmaPy, I'm considering dynamically creating nox sessions that run the tests for each subpackage. For example, doing nox -s particles would let me run the tests and doctests for plasmapy.particles.

I'm currently switching over to a nox session that regenerates pinned requirements files used in CI. It's really helpful to be able to use a for loop! I'm also adding a mildly helpful function to generates the names of the requirements files, which I'm likely to reuse in testing environments. This is in PlasmaPy/PlasmaPy#2664.

I've previously considered dynamically extracting the minimum requirements from pyproject.toml to run tests against minimum requirements, eliminating the need to define them in both pyproject.toml and tox.ini. (This is no longer necessary, as uv has a --resolution=lowest-direct flag to do this for us.)

My understanding of tox is that is already allows defining sessions by composition, which could be argued is the only kind of dynamism that we need. I'm personally worried that introducing more dynamism would inflate the maintenance burden, not reduce it.

I suspect that the amount that this effect would increase the maintenance burden would be more than offset by the reduction in maintenance burden by switching from an INI format to a Python API. I'm curious how the maintenance burden was impacted in other projects that have made the switch from tox → nox.

I don't think we want to alienate the ecosystem that bloomed from astropy, and there's been a consistent effort to keep workflows as similar as possible between repos. Moving astropy away from tox would likely mean that we'd need to help coordinated and affiliated packages do the same, and it sounds like a sizable effort.

Agreed; to me this is the most compelling of the reasons not to switch. As with everything in software engineering, there are tradeoffs! ⚖️ It's difficult to find a balance between consistency & stability of package infrastructure, and adoption of new tools that can improve developer experience.

Thank you again!

@eerovaher
Copy link
Member

There is a high enough probability that Astral releases something in the near future that we might want to use instead of tox, so I view the idea of replacing tox with nox right now as a needless distraction.

@astrofrog
Copy link
Member

For PlasmaPy, I'm considering dynamically creating nox sessions that run the tests for each subpackage. For example, doing nox -s particles would let me run the tests and doctests for plasmapy.particles.

FYI this currently works with tox by doing e.g.

tox -e test -- -P io.fits

which will run all tests in astropy.io.fits.

I suspect that the amount that this effect would increase the maintenance burden would be more than offset by the reduction in maintenance burden by switching from an INI format to a Python API. I'm curious how the maintenance burden was impacted in other projects that have made the switch from tox → nox.

I would say our maintenance burden for using tox.ini is very little right now - I haven't seen any real complaints about it in the last couple of years or things we have spent significant time to work around, but I might be wrong. Tox has also got a lot better with tox 4 as tox --develop is now nice and speedy.

@neutrinoceros
Copy link
Contributor

There is a high enough probability that Astral releases something in the near future

Were there any hint specifically in that direction ?

@eerovaher
Copy link
Member

There is a high enough probability that Astral releases something in the near future

Were there any hint specifically in that direction ?

tox is one of the tools they mention in a blog post. That's not saying very much, but it does qualify as a hint.

@nstarman
Copy link
Member

Switching to nox would lead to differences with packages whose architecture is inspired by Astropy's.

I'm also on the nox bandwagon. I switched recently when I started using https://github.com/scientific-python/cookie to set up my projects. We don't bake a cookie ourselves anymore, and I think this is the one we do / should recommend people use. Anyway, https://github.com/scientific-python/cookie uses nox and IMO it's much more pleasant than tox thus far.
So yes, we would have differences with packages whose architecture was inspired by Astropy's, but we would a) hopefully inspire them to move towards the https://github.com/scientific-python package layout and b) Astropy would be more similar to packages whose architecture is inspired by the scientific python community.

@namurphy namurphy changed the title [proposal] Switch from tox to nox for running continuous integration environments [proposal] Switch from tox to Nox for running continuous integration environments May 19, 2024
@astrofrog
Copy link
Member

Just to check, does anyone here have concrete examples of issues with the current tox set-up that would actually be solved by switching to nox? How often do people even have to delve into the current tox config?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants