add initial tox.ini #3196
add initial tox.ini #3196
Conversation
|
ugh, native toml not supporting https://tox.wiki/en/latest/config.html#conditional-settings makes it a non-trivial conversion. I think the way to get around that is with more explicit environments + https://tox.wiki/en/latest/config.html#string-substitution-references; but... that's for another day/person |
|
If you review I propose delaying further CI/ |
| mypy --platform linux | ||
| mypy --platform darwin | ||
| mypy --platform win32 |
There was a problem hiding this comment.
Shouldn't this use mypy_annotate?
There was a problem hiding this comment.
hm. Maybe, but mypy_annotate is less needed with tox supressing output of succesful commands and in general reformatting the output. One of the problems with check.sh was simply finding what it was that failed, which I think is what prompted the creation of it. But we don't want it when running outside of CI, so needs some logic to differentiate.
But if we don't use it here then it should just be removed.
I'll procrastinate on it a little bit, and see what the output looks like in practice. I suspect the end result is going to be removing mypy_annotate
|
@jakkdl thanks for pinging! Love that it's being worked on. I'll review shortly :) |
I'd vote for |
| - name: Check Formatting | ||
| if: matrix.check_formatting == '1' | ||
| run: | ||
| python -m pip install tox && |
There was a problem hiding this comment.
You may want to use tox-uv too. Though, it's okay to omit. Especially in the initial effort.
There was a problem hiding this comment.
not sure there's much, if any, speedup when we're only running a single environment
There was a problem hiding this comment.
I haven't compared them, but I'd imagine that provisioning that env would be faster. You don't separate the provisioning step in this PR, so I can't see how much it takes. But it's probably not important enough right now.
| [testenv:docs] | ||
| deps = | ||
| -r docs-requirements.txt | ||
| # base_python synced with .readthedocs.yml |
There was a problem hiding this comment.
We can also make RTD just call tox in a follow-up. This is what I do these days.
| base_python = 3.13 | ||
| set_env = | ||
| PYRIGHT_PYTHON_IGNORE_WARNINGS=1 | ||
| commands = |
There was a problem hiding this comment.
Let's also add --python-version= in follow-ups.
There was a problem hiding this comment.
python versions are specified in pyproject.toml for both tools
edit: ah you mean parametrizing the version
@CoolCat467 there's a plugin: https://github.com/tox-dev/tox-gh. It also demonstrates separating the invocation into preparation and testing. But I think this is something to look into separately, beyond the initial effort. I don't exactly like how the jobs are mixed up right now, but I believe this is out of the scope, so I choose not to start those discussions just yet. |
| # protip: install uv-tox for faster venv generation | ||
|
|
||
| [testenv] | ||
| description = "Base environment for running tests depending on python version." |
There was a problem hiding this comment.
@jakkdl is configparser able to handle the quotes well? I think this is the first time I see them in this context.. Does the output of tox l look good?
There was a problem hiding this comment.
...no 😅
❯ tox l
default environments:
py39 -> "Base environment for running tests depending on python version."
py310 -> "Base environment for running tests depending on python version."
py311 -> "Base environment for running tests depending on python version."
py312 -> "Base environment for running tests depending on python version."
I started out on a tox.ini because it's handy to have; it's not complete enough to close #3170, but it should be sufficient for #2699 once the CI is updated to use it.
noxfor that not to be a nightmare.tox.iniI didn't realize this until most of the file was written.ci.ymlto actually use this. We'd probably want https://github.com/tox-dev/tox-gh or https://github.com/ymyzk/tox-gh-actions to do that properlyeverybody's very welcome to push commits to this PR, but I don't think we should block merging it until the tox/nox config is perfect (leaving that for followup PRs) Cause I Wanna Use It ™️