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
Integrate tox #790
Integrate tox #790
Conversation
Can you point me at some good resources to learn about and use tox? |
Hmm, it's kind of second nature to me at this point but this looks like a good article to get you started https://opensource.com/article/19/5/python-tox. In short, tox creates a virtual environment (virtualenv) and installs the package plus whatever other dependencies you specify in it, before running the command. You use it with |
https://tox.readthedocs.io also helps. We should update |
I can do that in a follow-up to the doc cleanup series. Would you be okay with me just replacing all references to the manual script with tox invocations? |
5223553
to
1fece67
Compare
Interestingly, I get failures when running on the Mac. Will investigate why. |
What are the failures? I suspect you might be missing some interpreters. Does |
Full output here: https://gist.github.com/akrabat/0dfc4beb4d160549db1a5a2cde820322 |
There looks to be two issues here. Firstly, you're missing a Python 3 interpreter. If you run |
Currently I have two Pyenv environments: |
Can you try the following?
And share the output of each. |
In case it's not clear, I'm trying to figure out what the difference between the two environments. In theory, there shouldn't be one, but clearly something different is happening. |
Thanks for the help. Will have to be Sunday as I've been up since 6am and have stepped away from the laptop & won't be back home until then. |
No idea if this is the issue (please pardon the drive-by comment), but $ tox --recreate -e py37 |
Resolved the tox Python 3 problem. Seems that you need tox for Python 3, not tox for Python 2... |
pip freeze differences (side-by-side list):
Items in my env that aren't in tox:
|
Snip
Most of these are dependencies of tox.
This looks unnecessary now. We replaced this with
Again, most of these are tox-related. |
1fece67
to
2806cef
Compare
2806cef
to
398032c
Compare
This should be good to merge once #854 has landed and got rid of the based JSON testing, though it will need the PyMuPDF added as a dependency. |
398032c
to
05bd2db
Compare
05bd2db
to
3e14c22
Compare
@akrabat This is good to go now, I think |
I think tox hates me. This is a clean venv with 3.7.7 installed. All I have done in the venv is:
Running
Full output is here: https://gist.github.com/akrabat/433b30a5fcdccd1b182d8afce8ea92df These are the log files found in The output of:
is: https://gist.github.com/akrabat/9283281ec6243b600911dc97c753d27a For my pyenv virtualenv, I have this A cursory glance shows many differences. The most two suspicious are that tox has installed ReportLab 3.5.42 and Sphinx 3.0.4 even though Any idea why tox doesn't honour the |
Please try |
Didn't help. I'm going to try again with pyenv out of the equation… |
If I do:
it works. i.e. Inkscape doesn't crash and the right Sphinx is used. |
tl;dr: Sorry about that. Fixed. Instead of installing everything from The solution here is to either (a) use As for why I didn't spot this before? I had loads of failing tests before #854 and #870 merged and I missed the additional ones being added to the pile. 🙈 EDIT: You'll note I'm using the constraints approach in EDIT: I've probably linked this before, but good reading background reading here on my note above about reworking requirements to use ranges. See also discussion on |
This needs a lot more work including fixing the style check (which is failing *hard* right now), and finishing Python 3 support, but it gives us a framework to work with. Signed-off-by: Stephen Finucane <stephen@that.guru>
- Remove unnecessary imports - Stop checking for the presence of 'json' - it's been in stdlib since 2.6 [1] - Fix some formatting - Remove invalid 'test_suite' and unnecessary 'tests_require' config [1] https://docs.python.org/2/library/json.html Signed-off-by: Stephen Finucane <stephen@that.guru>
Signed-off-by: Stephen Finucane <stephen@that.guru>
3e14c22
to
283ed00
Compare
@@ -21,46 +36,16 @@ def read(*rnames): | |||
'setuptools', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this package need setuptools
as a runtime dependancy? Generally this wouldn't be in install_requires
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use pkg_resources
to get version information. To the best of my knowledge, that is not packaged separately and would require setuptools
. Right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, okay. I've usually seen it better to use one of these techniques than require setuptools
, though it's probably fine: https://packaging.python.org/guides/single-sourcing-package-version
Maybe importlib.metadata
package would be a more modern alternative? (Built into Python 3.8+).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be happy to switch to importlib.metadata
for Python 3.8+ with the importlib_metadata
package included as a dependency for Python < 3.8. If we did this, we should probably also integrate setuptools_scm
to automate versioning based on Git tag. I'd also be happy to just hard code the version and just use that (dumb approaches work too). Probably should do whatever we do settle on in a separate PR though as I'm mostly just cleaning up the setup.py
file in this PR.
|
We are dropping support for Python 2.7 shortly. Let's go with 3.7 instead. Signed-off-by: Stephen Finucane <stephen@that.guru>
I think However, I think that
(and then lots more errors) |
I'll read up on the constraints thing, but I agree with you about ranges. Hopefully, the updated test system will make that easier to do. |
Correct. I have a large PR waiting in the wings. Just need your py3 series to merge first.
tox isn't to blame here. Rather my changes in commit 48305e3 are. We're using the include directive and looking for multiquotes using EDIT nvm, fixed here. |
We're not running tests here and therefore don't need these. Signed-off-by: Stephen Finucane <stephen@that.guru>
In change 48305e3, we changed the module docstring to use double quote multiline comments instead of single quotes. The manual was expecting so correct things. Signed-off-by: Stephen Finucane <stephen@that.guru>
With a minor fix to the |
tox is the automation tool of choice within the Python ecosystem. Integrate it
and avoid the need to manually create a virtualenv to test things.