Add commas between args to passenv in tox.ini

[jlapeyre]

The presence of commas is enforced in tox 4.0.6

Closes #812

[coveralls]

Pull Request Test Coverage Report for Build 4149952451

• 0 of 0 changed or added relevant lines in 0 files are covered.
• 11 unchanged lines in 2 files lost coverage.
• Overall coverage decreased (-0.04%) to 97.041%

---

Files with Coverage Reduction	New Missed Lines	%
src/shortest_path/all_pairs_dijkstra.rs	2	98.54%
rustworkx-core/src/min_scored.rs	9	59.09%

Totals
Change from base Build 4137542295:	-0.04%
Covered Lines:	13873
Relevant Lines:	14296

---

💛 - Coveralls

[IvanIsCoding]

LGTM

[IvanIsCoding]

One thing I would like to ping @mtreinish before merging this is our policy for using Tox 4: https://github.com/Qiskit/rustworkx/blob/main/.github/workflows/main.yml#L87

Right now we use Tox 3. Clearly this change passes the CI on Tox 3. But I wonder if we should profit from this to upgrade to use Tox 4

[jlapeyre]

Is tox < 4 specified elsewhere? It would be useful for developing if it were mentioned in say CONTRIBUTING.md. Or some Python requirements file, rather than just main.yaml.

[IvanIsCoding]

Can you revert #761 and bump Tox to 4.0 in CI? Then we can check that it works with the latest version which most people will start installing

[jlapeyre]

Yes, I can do that. I got the latest, 4.0.6, and the only problem locally was that this comma thing. Don't know about CI.

[jlapeyre]

CI failed. I wonder what ERROR: Operation cancelled by user means.

[IvanIsCoding]

Yes, I can do that. I got the latest, 4.0.6, and the only problem locally was that this comma thing. Don't know about CI.

The Ubuntu retworkx build is failing, I am going to investigate why it worked with Tox 3 but not 4

[jlapeyre]

The Ubuntu retworkx build is failing, I am going to investigate why it worked with Tox 3 but not 4

I 'm digging through the logs

[jlapeyre]

Looks like testing fails. But there is a lack of detail. Can't see the python stack trace.
I do see subprocess-exited-with-error, which makes me think it might be #811 EDIT: No, I agree with @mtreinish below, this is unrelated.

[mtreinish]

We'll likely need deeper changes to the tox configuration to enable it to work with tox >4.0.0. Tox 4 made a number of breaking changes, including critically how the build process works. I capped it to tox<4.0.0 at the time to give tox a chance to stabilize after their big breaking release before circling back to update the configuration. So far I don't think they've really slowed down making releases looking at https://pypi.org/project/tox/#history but I agree we should remove the cap since people are getting tripped up on this.

The relevant section in the upgrade guide around where I expect the issues are: https://tox.wiki/en/latest/upgrading.html#packaging-environments

[mtreinish]

Looking at the failure in particular it seems to be with the retworkx backwards compat side: https://github.com/Qiskit/rustworkx/blob/main/.github/workflows/main.yml#L93-L131 it looks like tox isn't happy trying to build the retworkx package like it was in tox 3 just by setting the env variable

[jlapeyre]

we should remove the cap since people are getting tripped up on this.

Yes, but if I'm not mistaken we don't mention anywhere that you need tox 3. It's only in the CI config files. And the instructions we give are explicitly incorrect pip install -U tox.
Furthermore, isn't it possible to put the version restriction in requirements.txt or setup.py, etc. ?

[mtreinish]

we should remove the cap since people are getting tripped up on this.

Yes, but if I'm not mistaken we don't mention anywhere that you need tox 3. It's only in the CI config files. And the instructions we give are explicitly incorrect pip install -U tox. Furthermore, isn't it possible to put the version restriction in requirements.txt or setup.py, etc. ?

It was an oversight, the tox pin wasn't meant to be there long term, but it was intended to be a short term workaround to unblock CI. But it's ended up persisting longer than I expected and the level of compatibility between tox 3 and tox 4 has decreased even further since the earlier releases in the tox 4.x series.

There isn't a good way to specify a tox version cap automatically since tox triggers the package build and installs the requirements and test requirements for us. If we put tox<4 in the requirements-dev.txt list that would end up having tox install tox in the test venvs it creates. Similarly, putting it in one of the other packaging files like setup.py or pyproject.toml would install tox too late because it would either get installed during build or install time for the rustworkx package (which is both not desired and also too late since tox is what installs rustworkx in a venv for local testing)

[IvanIsCoding]

Closing after #851 got merged