riscv-opcodes
riscv-opcodes copied to clipboard
Reorganise Python code using pyproject.toml
Add pyproject.toml (the modern alternative to requirements.txt), making this a proper Python package that can be installed via pip and potentially uploaded to PyPI.
This required moving all of the data files into the package, and reading them using importlib. This gives a much nicer file structure. It does require Python 3.7 but that is over 6 years old and easy to install on old Linux distros like RHEL 8.
Note: This is a draft; I haven't quite finished the bit that actually reads the opcode files, but I wanted to get feedback to see if you're ok with this approach first.
Especially the bit about creating a venv. Python's native tooling is kind of terrible so this is not easy to avoid. If you install rye then it becomes a bit better; you can run rye run riscv_opcodes ... and it will take care of the venv automatically. Also end-users won't see the pain - if they do pip3 install riscv_opcodes then they will just be able to run riscv_opcodes ....
Solution to a non-problem IMO.
We use riscv-opcodes in our Python flow and it's extremely awkward to use as-is. It would significantly easier if we could just install it as a package.
Could I get you to reconsider? Python tooling is awful at the best of times and going against modern Python standards just makes it even worse.
Especially now that uv exists... this would solve annoying issues like this:
$ make
Traceback (most recent call last):
File "/home/me/riscv-opcodes/./parse.py", line 16, in <module>
from svg_utils import make_svg
File "/home/me/riscv-opcodes/svg_utils.py", line 5, in <module>
from matplotlib import patches
ModuleNotFoundError: No module named 'matplotlib'
make: *** [Makefile:18: everything] Error 1
Yeah, sure, I'll reconsider if we do it in a way that doesn't reorganize the directory structure of the extensions files themselves.
I updated the PR so they are in the same layout - the extensions directory is moved but otherwise unmodified. Is that acceptable?
Actually I might be able to do it without moving them at all... let me try.
Cool, let me know when it's time to look at the PR again.
Will do.
Codecov Report
:x: Patch coverage is 96.93878% with 3 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 97.08%. Comparing base (383cbca) to head (77b33c9).
:warning: Report is 37 commits behind head on master.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| src/riscv_opcodes/shared_utils.py | 95.12% | 2 Missing :warning: |
| src/riscv_opcodes/resources.py | 93.75% | 1 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## master #294 +/- ##
==========================================
+ Coverage 96.53% 97.08% +0.55%
==========================================
Files 10 14 +4
Lines 750 926 +176
==========================================
+ Hits 724 899 +175
- Misses 26 27 +1
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Sorry for the noise - difficult to debug CI failures locally.
Anyway it finally passes and I think it's pretty good. A minor hack to make it work with the original extensions paths but it's not too bad. CI now generates a wheel which could be uploaded to PyPI. Then using it would be as simple as this:
uvx riscv_opcodes -c 'rv*'
(Assuming you already have uv installed, which is a one-liner.)
Sorry @Timmmm, I missed that you had fixed this up. I rebased it manually since I introduced a merge conflict in the meantime. See #378.
Thanks!