-
Notifications
You must be signed in to change notification settings - Fork 119
Make Python init more language native #600
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
At this time there are a couple of issues I know I need to address:
|
xdelaruelle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Many thanks for this pull request. This is already in a very good shape. See review comments for the things to adapt.
Regarding tests:
- the
script/mtutility will guide you to locate where the test failures come from - a
grep -R setbinpath testsuite/run at the root of the repository will help you to see where new tests should be added (you may largely inspire from these setbinpath tests)
0639451 to
a6b1156
Compare
|
I've addressed the issues noted above except the failing tests. I still need to:
|
Renaming python.py to env_modules.py. On Linux, symlink python.py to point to env_modules.py for backward compatibility. Windows doesn't typically support unix-style symlinks, but we don't need to worry about that since the Windows installations of Environment Modules never had python.py. Signed-off-by: Byron Boulton <[email protected]>
Signed-off-by: Byron Boulton <[email protected]>
Adding the initdir to PYTHONPATH allows users to "import" the "module" function in Python scripts. Signed-off-by: Byron Boulton <[email protected]>
a6b1156 to
9ad195b
Compare
xdelaruelle
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see my comments for the few changes to apply.
Signed-off-by: Byron Boulton <[email protected]>
Signed-off-by: Byron Boulton <[email protected]>
Signed-off-by: Byron Boulton <[email protected]>
Signed-off-by: Byron Boulton <[email protected]>
|
I pushed up some fixes. I want to rebase/squash them, but I need to sign off for now. |
This pull request is an effort to implement a more language-native way to get the "module" function in Python scripts. It is the Python portion of #483. I first proposed this pull request in the mailing list. Below is a description of the plan from that conversation.
For clarity I'll use "Python-module" to refer to a Python script defining functions/classes that you
importin Python code and "environment-module" to refer to a TCL environment module that youmodule load/module unload/etc.The current documentation for Python initialization (https://modules.readthedocs.io/en/latest/module.html#examples-of-initialization) has
A more Pythonic approach would be to
importthismodulefunction somehow. Currently, you can do that with as followsThe issues with this are that you have to modify the
sys.pathinside the script and since the Python-moduleimport-ed is called "python". The name makes it look likemoduleis a function from some core Python-module from the Python project.In this pull request I will
env_module.py(this is what lmod named theirs by the way) andtcl/subcmd.tcl.inadd the folder of initilization scripts to the PYTHONPATH environment variable.These changes would change Python initialization to
which cleans up the way the Python script imports the
modulefunction and communicates more clearly that it comes from a 3rd-party Python-module related to environment modules.