[WIP] ModuleNotFound Error Fix #115
Conversation
tests/test_output.py
Outdated
| def test_install_and_import(tmpdir): | ||
| with tmpdir.as_cwd(): | ||
| kit = CookiecutterMDAKit(template_analysis_class="MyAnalysisClass") | ||
| kit.run() | ||
| result = subprocess.run(["python", "-m", "pip", "install", | ||
| "-e", "./test-mda-kit"], | ||
| capture_output=True, text=True) | ||
| if result.returncode != 0: | ||
| pytest.fail(f"Failed to install: {result.stderr}") | ||
| try: | ||
| import test_mdakit_package | ||
| except ImportError as e: | ||
| pytest.fail(f"Failed to import 'test_mdakit_package': {e}") | ||
|
|
||
| def test_gh_actions_debug_python_env(tmpdir): | ||
| with tmpdir.as_cwd(): | ||
|
|
||
| result = subprocess.run(["python", "-m", "pip", "install", | ||
| "mdanalysistests"], | ||
| capture_output=True, text=True) | ||
| if result.returncode != 0: | ||
| pytest.fail(f"Failed to install: {result.stderr}") | ||
| try: | ||
| import MDAnalysisTests | ||
| except ImportError as e: | ||
| pytest.fail(f"Failed to import 'MDAnalysisTests': {e}") |
There was a problem hiding this comment.
Thanks so much for opening this PR @ljwoods2! For the cookiecutter repo we have a somewhat complex structure for tests, and in many ways I've found it easier to just run the "cookie" tests as part of a GitHub action (e.g. in #116 I'm currently trying to get the "cookie" tests to fail appropriately, as part of the cookie testing is importing the package). This also avoids complicated actions like installing a Python package into the active environment while Python is running, which can also potentially complicate other tests in the same environment. I'd be keen to err on the side of keeping this kind of importing test to within the cookie infrastructure as much as possible, as it also replicates the user experience more (at least when the tests work...)
There was a problem hiding this comment.
Ah, I see, thank you for explaining, I thought the issue was that the install wasn't being tested, not that the test was failing out before it installed and imported the package. I will remove this
| @@ -6,7 +6,7 @@ requires = [ | |||
| build-backend = "setuptools.build_meta" | |||
|
|
|||
| [project] | |||
| name = "{{cookiecutter.repo_name}}" | |||
| name = "{{cookiecutter.package_name}}" | |||
There was a problem hiding this comment.
Hmm -- the docs reckon this name should be what you want the package to be called on PyPI, and allow things like hyphens here. Should this remain just the package name? @orbeckst any opinions here -- what did you have in mind for mdgeomkit?
Edit: sorry I see you reverted this later!
There was a problem hiding this comment.
Not sure if my input is still needed but for completeness: I just tried a different name for the project_name (the PyPi install name, the one that can have hyphens) from the Python package/module "import" name package_name to see if anything would break... and it did:
project_name [ProjectName]: mdgeomkit
repo_name [mdgeomkit]: mdgeomkit
package_name [mdgeomkit]: mdgeom
There was a problem hiding this comment.
LGTM and now fixing the broken CI in main -- thank you @ljwoods2! Just one final nitpick and please add yourself to AUTHORS: https://github.com/MDAnalysis/cookiecutter-mdakit/blob/main/AUTHORS.md
tests/test_output.py
Outdated
| import subprocess | ||
| import sys |
There was a problem hiding this comment.
I think these imports now aren't used?
|
Thank you @ljwoods2 for fixing this issue! |
|
Many thanks @ljwoods2 and @lilyminium ! |
Fixes #113
Changes made in this Pull Request:
I used a repo which contains multiple packages as a model for these changes: https://github.com/Gallopsled/pwntools
The idea is that if the repo name differs from the package name, you should be able to do
But not be able to import the package using the reponame. You must import using
However, this leaves the issue of versioning, since the package_name's
__init__method uses (AFAIK) the version is associated with the repo_name. I set the package's version to use the version associated with repo_name by default and the user will have to be responsible for changing this if it doesn't apply (in the case they have two packages in the repo that are both different versions)PR Checklist