Python: lazy imports and extras - #669
Conversation
Move `import pandas`, `import networkx`, `import graphviz`, `import IPython` and `import tabulate` from module top-level into the methods that use them (`Nfa.to_dataframe`, `Nfa.to_networkx_graph`, `plot_using_graphviz`, `display_inline`, `BinaryRelation.__str__`). Why: - `import libmata.nfa.nfa` was pulling pandas+numpy+networkx (+~888 modules, ~50 MB RSS); `import libmata.plotting` added IPython (+~600 modules, +28 MB). After this change, both imports add 0 MB / ~20 modules; the heavy deps are only loaded if the corresponding feature is actually used. - The eager pandas import also fed into a hang we hit in a downstream pytest fixture that patched `datetime.datetime` with a subclass before pandas had been initialised. Lazy loading sidesteps that interaction. Behaviour is unchanged for any caller that exercises these methods. Type hints in the affected method signatures are switched to forward-string form so they no longer require the module at definition time.
Reorganise `bindings/python/pyproject.toml` so that:
- Core libmata has no runtime Python dependencies (`dependencies = []`).
- Optional integrations are exposed as fine-grained extras:
* `libmata[pandas]` -> pandas (for `Nfa.to_dataframe`)
* `libmata[plotting]` -> graphviz, networkx, ipython, tabulate
* `libmata[all]` -> pandas + plotting
- Dev / test tooling moves to `[dev]` extra (pytest, pytest-cov, coverage).
- Example-notebook deps move to `[notebooks]` extra (ipykernel, papermill,
seaborn).
Update `bindings/python/Makefile`'s `init` target to `pip install
'./[all,dev]'` so the existing `make init && make test` flow keeps
working in CI and locally.
NOTE: this is a breaking change for users who relied on `pip install
libmata` pulling pandas/networkx/etc. transitively. They should now
install `libmata[all]` (or the narrower extra they actually need).
Combined with the preceding lazy-import refactor, a minimal
`pip install libmata` now ships only the C++ bindings; the heavy
Python deps are opt-in.
54ba55e to
8a15d26
Compare
The previous `make install` ran `python setup.py build_ext -fi`, which built `.so` extensions in-place inside `bindings/python/libmata/`. Tests run from `bindings/python/`, so Python's CWD-precedence picked up that local `libmata/` directory and resolved its submodules (alphabets, parser, etc.) from the in-place `.so` files. After switching `make install` to `pip install ./` (PEP 517), pip built the wheel out-of-tree and installed it to site-packages -- but the local `bindings/python/libmata/` source directory still shadowed the site-packages install, and no `.so` files were placed there, so tests failed with `ModuleNotFoundError: No module named 'libmata.alphabets'`. Switch both `make init` and `make install` to `pip install -e .` (editable). Editable mode builds extensions in-place, restoring the old `.so`-in-source-tree layout while still using PEP 517 (so the `[build-system] requires` Cython gets pulled into the build env automatically).
`make build-dist` / `make pypi-release` ran `python setup.py sdist`,
which bypasses PEP 517 and so doesn't honour `[build-system] requires`
in pyproject.toml. Since Cython is no longer installed at runtime, the
naked `setup.py` import failed in CI:
ModuleNotFoundError: No module named 'Cython'
Switch to `python -m build --sdist`, which builds in an isolated env
populated from `[build-system] requires`. `make pypi-release` now
delegates to `make build-dist` so the two targets stay in sync.
Use `python -P` (Python 3.11+) so that the local setuptools `build/`
artefact directory in this folder doesn't shadow the pip-installed
`build` package on sys.path.
|
@Adda0 can you please take over the review as I'm not aligned with the Mata anymore? Or delegate this to someone. Just my two cents: the imports should always be topmost, never hidden and imported in functions; if the modules are used for typing hints only, you can use There is also official support of lazy loading in Python, but that is AFAIK either suported from 3.14 or 3.15, which mind be too big version restriction. |
|
Of course. I do not know why your review has been requested. Is there a specific reason for the tag, @p4l1ly ? |
|
no, sorry, I've just clicked on something like "request recommended reviewers" |
|
I'd say there's not even a need for lazy loading here. It could be something like "conditional imports" of some functions based on presence of library... I can rework, only tell me if you want it and if this feature is acceptable for you. |
Make the module footprint of the Python library minimal by default, use optional dependencies for non-core utilities.