Add a JSON output for pyodide xbuildenv search, better tabular output#28
Conversation
|
Also, I got slightly nerdswiped by the tabular outputs we have, and implemented a prettified version of it (from https://ozh.github.io/ascii-tables/) in ce1904a, see below:
|
|
The only thing left for me to do here is check with |
pyodide xbuildenv searchpyodide xbuildenv search, better tabular output
|
I've tried this with This should help set up a base for developing wheels against arbitrary Pyodide versions in In any case, this is ready for review – if we need a better output from this command, we can always implement improvements to it in subsequent PRs and versions. |
|
Also, the context manager fixes our problem that was present in pyodide/pyodide#4814. |
My goal is to prevent breaking changes as much as possible, but yes, some changes are inevitable. If any breaking change happens, I'll basically update the The problem at this point is that when people say they want to build a package that targets an older version of Pyodide, they don't know which version of pyodide-build to install. What people can do now is to install pyodide-build first, check for compatible versions with |
ryanking13
left a comment
There was a problem hiding this comment.
Thanks! I left some comment.
I have a few thoughts in mind:
I think it all boils down to the Pyodide version support we can offer in a realistic manner. For example, we should make it explicit that someone should not use, say, Pyodide version 0.21 in 2024 (just an example). The Pyodide version is also influenced by the versions of the packages present in it, because they would have to be compatible with the CPython version in Pyodide. This symbiotic relationship between the Pyodide, CPython, and the Emscripten versions is what makes Pyodide work as a full-fledged distribution, but it doesn't offer guarantees on release and support cycles. With the Scientific Python ecosystem having adopted SPEC 0000 for bumping minimum versions of dependencies and minimum bounds of CPython, if we have our own consensus on a minimum supported Pyodide version in the release cycle (please let me know if we have one already!), we can better gauge a solution for Either way, a Markdown table in the README could be added (from the third point). |
|
The integration tests seem to fail to run on my fork, but that is not the case here. I triggered them here so that there are no regressions later on. |
Yes, I think this is a most doable solution that does not increase the maintenance burden. We can probably also provide a file that specifies the maximum compatible pyodide-build version for each version of Pyodide (actually, our metadata file can already be used for that feature).
Actually, we don't have any support for older Pyodide versions. If something breaks in older Pyodide, we just ask them to "update the Pyodide" (or pyodide-build). But yes, as now pyodide-build is unvendored and people start to build packages with cibuildwheel, we need to consider it... |
ryanking13
left a comment
There was a problem hiding this comment.
Thanks! I left some minor comments.
| @pytest.fixture() | ||
| def is_valid_json(): | ||
| def _is_valid_json(json_str): |
There was a problem hiding this comment.
Question: Why is it defined as a fixture? Doesn't calling is_valid_json work?
There was a problem hiding this comment.
Oh, I was using it in the test invocation, which meant that it needed to be a fixture for pytest to recognise it. But we don't need that, necessarily. In a9f9a61, I've removed the fixture and called it directly, as you suggest.
Thanks for your thoughts on this. Yes, linking to the metadata file makes sense, too (it's just that a formatted table is more readable with the same data). I guess we should open a new issue about this, because it is also tied in with, say, including multiple versions of packages in the same Pyodide version (useful for interactive documentation across versions). I guess I need to write a detailed RFC issue about that prospect... |
|
I'll put up a reminder about thinking more about this, I'll hopefully write something after PyCon this week. In other words, I guess this is ready to merge, now. Thank you for the review! |
Description
This PR closes #26. It adds a
pyodide xbuildenv search --jsonoption to print a JSON-based output, along with associated tests. The advantage is that it can be saved to a file, piped tojqor shell functions (or any equivalent tools), or simply imported into a Pythonic interface.Additionally, I added a small context manager that does not do anything but stop printing the following lines:
in the output, because it conflicts with receiving valid JSON. Please let me know if this would be undesirable. If yes, I'll try to work around it so that it gets printed for the non-JSON output (table).