Support for Automatic <py-env> with dependency + few tweaks to contributing and readme docs #36
Conversation
The new module extends the ModuleFinder from find-imports (in graveyard) to detect all the dependencies of the file being converted. Currently the parser is able to collect either external packages (limited to those supported by Pyodide) and any local module to be included in `paths`. Local submodules (node.level > 0) not yet supported.
Code parsing has been completely refactored and improved to support the identification of corner import conditions that won't work in Pyscript. This includes unsupported packages, and local modules/packages import that won't work if not imported directly. As a result, ModuleFinder gathers all the packages via NamespaceInfo (populated via pkgutil) and collects results in a FinderResult object. This object will be later used to specialise the processing result.
find_imports functions now leverages directly on the FinderResults class to encapsualte parsing results.
Warning message is now generated (in yellow) when the converted Python module contains unsupported packages or local modules (as in packages)
nbconvert is an extra dependency required to support the conversion of notebooks into python files. Pre-commit on the other hand was a potentially missing dependency for dev. Last but not least, requests and six are also needed to be included otherwise those will be removed at every `poetry install`
Also added missing dependencies
Also added missing dependencies
and removed useless code from prev. manual merge
this commit includes a few general abstractions to ease the testing of cli with multiple py-code snippets. In particular, the default py-code (no imports) has been wrapped into fixture, and all tests changed accordingly to avoid hard-coding py-code. Moreover, a new py_code_with_import fixture has been added to support testing of the new py-env integration feature. A new test has been added, having multiple dependencies dynamically injected via `Dependency` dataclass.
ipykernel dependency is required by nbconvert to convert notebooks into Python code files.
for more information, see https://pre-commit.ci
…cli into feat_pyenv_support
for more information, see https://pre-commit.ci
|
@leriomaggio Sorry, I have forgotten to configure [tool.isort]
profile = "black" |
…cli into feat_pyenv_support
YES! that was giving me headaches already!! will do :D |
Tried to update (as per instructions from poetry#4983 |
I'm not sure but haven't had a chance to really look yet. I believe they just released poetry v2 so it's quite possible there were some API changes. If we are doing a blind I will take a look later today when I get a chance. |
So what I did was essentially changing (manually) the requirement for However, looking at setup in github actions, |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
Codecov Report
@@ Coverage Diff @@
## main #36 +/- ##
==========================================
- Coverage 96.78% 94.06% -2.73%
==========================================
Files 10 12 +2
Lines 249 438 +189
==========================================
+ Hits 241 412 +171
- Misses 8 26 +18
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
|
@leriomaggio I believe the I also cleaned up some type hints. I will give the rest of the code a more thorough review later, but I wanted to get the CI passing for you. Gotta hone this automation (or throw it away if it is too onerous for contributors). |
|
@mattkram thanks very much for taking the time to have a look at that - and for making the automation to pass. Re I think the question is whether we want to have these groups, or to keep it simple? Or maybe move that one to So far the As for the other deps in |
|
FWIW, in the plugins we're developing (with poetry) we're using the new groups based dependency schema as per poetry 1.2. |
There was a problem hiding this comment.
The code looks great. Nice job. I honestly don't have anything to add to nor change about it. 👍
Now comes the interesting bit, the way we specify dependencies is in a state of flux and what we have in this PR is going to be out of date very soon.
- In projects (made by the
createcommand) dependencies will be listed in amanifest.tomlfile. - The HTML pages inside the project will reference this
manifest.tomlfile, and may additionally have apy-envtag containing additional dependencies or package overrides. This inner-text of this tag will be TOML (not YAML as it is now). - For non-project, single page, PyScript work (as created by the
wrapcommand) thepy-envwill simply contain a TOML specification of the dependencies.
While I've approved this PR, FIRST we need to decide (all input welcome!):
- Merge this (soon to be changed) work now?
or - Wait until the config/env/toml work is settled down and merged into PyScript core before merging this work (modified to match the new schema).
|
Thanks @ntoll for the review and for finding the time to go though the code! 😊
This is indeed a very good point! Just throwing a couple of points here that could hopefully give food for thoughts to help deciding about this (or not?! 😅 )
HTH |
|
@leriomaggio Seems like this PR was forgotten for a long time 😬 |
This PR finalises the new
pyscript-clifeatures developed during the last hack-days.In particular, the new features this PR brings are:
<py-env>tag in the html template.pathsas per PyScript naming..py) and Notebooks (.ipynb) could be passed in as input and converted.Note: from hack-days version, this code has been merged and aligned to support the new plugin-hook design/feature, plus - more importantly - a few tests for the new features have been included.
On this note, a quick refactoring has been done to
test_clito include fixtures for code, with relatively simple injectable dependencies.Other changes in this PR concerns:
CONTRIBUTING.mddocumentation providing instructions on how to setup dev environment (not just for docs)poetrydependencies have been revised and updated, includingpre-commitas dependency, with instructions in the main doc on how to install hooks.README.mdto showcase input code from both python and notebooks files.