Use relative imports everywhere in src/smlp_py and conditionally in run_smlp.py

[fbrausse]

In order to switch to a proper Python package that can be imported and run without the concrete source tree directory structure, we need relative imports. Otherwise "from smlp_py import ..." wouldn't work. The current setup is centered around the assumption that src/run_smlp.py is the main and only entry point. That assumption is no longer valid for Python packages.

[mdmitry1]

I don't understand why it is needed.
For Linux everything works fine.
Please, clarify

[mdmitry1]

Please, build a wheel and run regression before I'll start code review

[fbrausse]

This PR is against the master branch. As far as I know, it doesn't support wheels, yet: there is neither pyproject.toml nor setup.py.

[fbrausse]

Please, build a wheel and run regression before I'll start code review

How would I run the regression tests using a wheel built from #53? I'd need a way to execute run_smlp.py. However, the wheel doesn't provide that option - unless you manually invoke .../lib/python3.*/site-packages/smlp/run_smlp.py, which isn't what people expect to do for running a program.

Hence, in a44b1d2 we added a command called smlp for this wheel to be installed by pip. In order for it to work, the imports need to be relative.

[konstantin-korovin]

good

[mdmitry1]

In order to run regression test for PR #53, install wheel in virtual environment:

1. Install package and run dead-or-alive test

cd package_test/python3.11/venv
./run_dora

2. Enter virtual environment and run regression script

source smlp_package_venv/bin/activate
cd smlp_package_venv/smlp/regr_smlp/code
./smlp_regr.py [<regression script_parameters>]

For example:

source smlp_package_venv/bin/activate
cd smlp_package_venv/smlp/regr_smlp/code
./smlp_regr.py -t 1 -g

These instructions are contained in package_test/python3.11/venv/README.md

[fbrausse]

Unless I'm mistaken, this will not test the actually installed pip package since ../../src/run_smlp.py is hard-coded in the regression script smlp_regr.py.

[fbrausse]

Here is a run of #53 with this PR merged: pr53-with-pr56.log

And here is a run of #53 without this PR: pr53-without-pr56.log

They both pass. This PR is independent of #53.

[zurabksmlp]

Hi Franz

If you look into nlp_text.rabased branch, there we have other ways to run SMLP such as via an agent or GUI. Will you suggested change (with the imports) work also with these ways to invoke SMLP? The agent related code is in the src directory, just like run_smlp.py. Do you have an opinion whether the src directory is the right place for these "top level" scripts?

[fbrausse]

There should be no issue with adding new scripts to the src/ or any other directory. The point of this change: When SMLP is installed as a "wheel" by pip, the package will be smlp with a sub-module smlp_py. At that point, import smlp_py (absolute import) will no longer work for SMLP-modules imported by user code, because Python won't be able to find smlp_py. User code here also refers to executables we export in our pip-package. So if those nlp_text top-level scripts are supposed to be run by users, they should either do a relative import or a non-standard way of exporting those scripts needs to be designed.

[zurabksmlp]

What is the following comment about? When and why the if branch should go away?

if name == 'main':
# TODO: this branch should go away: replace by invoking the 'smlp' script
# which loads this file as a module
from smlp_py.smlp_flows import SmlpFlows
main(sys.argv)
else:
from .smlp_py.smlp_flows import SmlpFlows

[fbrausse]

This if-then-else contains the absolute import first, enabling to invoke this script from the terminal via, e.g., ./run_smlp.py. The else-branch has the relative import and is used when this script is imported from elsewhere, e.g., via from smlp.run_smlp import main. Ideally, in my opinion, we would not be requiring the __main__-functionality anymore and invoking SMLP would be done entirely through Python's module import system. However, this PR isn't trying to do that, yet. And if there are good reasons to keep direct invocation of this script, I can remove this comment. Please let me know...

[zurabksmlp]

Thanks for clarifying. In my opinion the option of running SMLP using run_smlp.py is useful at least for developers and maybe for some advanced users as well, and it should stay. Regrading the comment, I myself leave comments in code about possible improvements and mark then as TODO. In this case, it is likely undesirable that is such a short script that user runs (and can look into it) we will have a TODO like comment, so I would recommend to drop it.

Regarding the rest, I have checked the changes and also run locally a few tests. I think this is enough for me to approve the PR, more so that we currently do not have fully functional regression. But I will still run regression just to see there are no crashes and then will approve this PR.

[fbrausse]

Thanks Zurab, I removed the comment.

[konstantin-korovin]

Just a thought would setting PYTHONPATH resolve such issue ?

…

On Sat, 14 Mar 2026 at 16:59, Franz Brauße ***@***.***> wrote: *fbrausse* left a comment (SMLP-Systems/smlp#56) <#56 (comment)> Thanks Zurab, I removed the comment. — Reply to this email directly, view it on GitHub <#56 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIVYHTKYU5CCQ2XV4XNEFTT4QWFYRAVCNFSM6AAAAACWONZDEWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DANRQHA4DIMZRGA> . You are receiving this because your review was requested.Message ID: ***@***.***>

[fbrausse]

Just a thought would setting PYTHONPATH resolve such issue ?

Not quite. There are differences between running

1. python run_smlp.py
2. python -m smlp.run_smlp
3. python -c 'from smlp.run_smlp import main'

In cases 1 and 2 __name__ == '__main__' is true, in case 3 it is not. In cases 2 and 3, __package__ is set to smlp. As soon as it is, the absolute imports of smlp_py won't work (unless the cwd is the src/ directory). So this if here covers cases 1 and 3. For 2 one would probably need to examine __package__, would need to check. My goal here is that case 3 succeeds because that is the easiest way for user-code to interact with the smlp wheel package.

However, I was thinking to eventually replace ../../src/run_smlp.py in the regression script with something like case 2 or 3. When that happens, I believe we can easily switch between a pip-installed smlp package and the repo (for testing some current development) by setting the PYTHONPATH.

[konstantin-korovin]

Thanks for explanations Franz! That's helpful.

…

On Sat, 14 Mar 2026 at 17:38, Franz Brauße ***@***.***> wrote: *fbrausse* left a comment (SMLP-Systems/smlp#56) <#56 (comment)> Just a thought would setting PYTHONPATH resolve such issue ? Not quite. There are differences between running 1. python run_smlp.py 2. python -m smlp.run_smlp 3. python -c 'from smlp.run_smlp import main' In cases 1 and 2 __name__ == '__main__' is true, in case 3 it is not. In cases 2 and 3, __package__ is set to smlp. As soon as it is, the absolute imports won't work (unless the cwd is the src/ directory). So this if here covers cases 1 and 3. For 2 one would probably need to examine __package__, would need to check. My goal here is that case 3 succeeds because that is the easiest way for user-code to interact with the smlp wheel package. However, I was thinking to eventually replace ../../src/run_smlp.py in the regression script with something like case 2 or 3. When that happens, I believe we can easily switch between a pip-installed smlp package and the repo (for testing some current development) by setting the PYTHONPATH. — Reply to this email directly, view it on GitHub <#56 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AIVYHTOL5EMCX3XDCFJUZAT4QWKKFAVCNFSM6AAAAACWONZDEWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DANRQHE2TINZQHE> . You are receiving this because your review was requested.Message ID: ***@***.***>

[zurabksmlp]

I have inspected the changes and also run individual test and the full regression. I see regression diffs that are explainable and cannot be caused by the changes, I therefore approve the PR.