FAIRmat-NFDI / cookiecutter-nomad-plugin

MIT License
1 stars 2 forks source link

module and package names #17

Open aalbino2 opened 3 months ago

aalbino2 commented 3 months ago

If I set the package name and the module name to be different, the plugin will not be loaded.

Indeed at line 1091 of the file nomad/config/models/config.py, the package_name variable is not the correct one

@hampusnasstrom @blueraft

blueraft commented 3 months ago

Can you share the generated cruft.json file contents?

blueraft commented 3 months ago

I am able to reproduce this for this repo you've created.

I think my advice here would be to not set different names for the plugin_name and module_name. The prompt will autofill the correct module_name based on the plugin_name you've set.

  [1/12] full_name (John Doe):
  [2/12] email (john.doe@physik.hu-berlin.de):
  [3/12] github_username (foo):
  [4/12] plugin_name (foobar): my-plugin-name
  [5/12] module_name (my_plugin_name):

After entering the plugin_name, just press Enter and the correct module_name should be filled.

aalbino2 commented 3 months ago

Hey Ahmed, thank you. What do you think about erasing completely the possibility to prompt a module name?

It led me to this mistake so I guess also other people can do the same.

Also, I did not realize that the name in parenthesis was the already filled name, I though it was just showing which package I was in while setting the module name

blueraft commented 3 months ago

Good point about the mistake there. I'll see if removing the option has any effects on the cruft update mechanism, if it does maybe we have to change the message so that it's clearer you don't have to enter anything as the module_name.

hampusnasstrom commented 3 months ago

@lauri-codes do we really assume the the package name has to be the same as the module name (with "-" replaced b "_"). I think this is a bit odd. What if someones package contains multiple top level modules? The python import works fine, I'm not sure where you make the assumption about the package name based on the module name but I guess it's on the lines Andrea mentioned above.

Here is the error you get when trying to run the appworker and showing that it works in python for completeness:

importlib.metadata.PackageNotFoundError: nomad_my_plugin

  - thread_name: MainThread
  - timestamp: 2024-05-23 14:21.44
(.pyenv) [hampusnasstrom@Fair-5430-01 nomad]$ python
Python 3.9.18 (main, Aug 28 2023, 00:00:00) 
[GCC 12.3.1 20230508 (Red Hat 12.3.1-1)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from nomad_my_plugin.schema_packages import mypackage
>>> 
blueraft commented 3 months ago

I believe it's something setuptools does, well more inside the src folder, the python packages are placed under the module_name directory.

What do you get when you run the following?

from importlib.metadata import version, metadata
metadata('nomad_my_plugin')
hampusnasstrom commented 3 months ago
<email.message.Message object at 0x7f18756cb4c0>
blueraft commented 3 months ago

That's` a bit weird, cause in nomad we run the same code

https://github.com/nomad-coe/nomad/blob/7026f3fb8a0901c5b96ae2ceb623a312c992e28b/nomad/config/models/config.py#L1091-L1092

But you get a PackageNotFoundError there but not when you run in terminal.

hampusnasstrom commented 3 months ago

I changed back and forth between the working name in between and it seems like the appworker is running now. Could be something cached maybe...

lauri-codes commented 3 months ago

@lauri-codes do we really assume the the package name has to be the same as the module name (with "-" replaced b "_"). I think this is a bit odd.

I don't really follow: I thought that the cookiecutter asks you about the repository name (by convention should be something like nomad-mypackage) and then the python package name (by convention should be something like nomad_mypackage). These two names can be completely different though. I don't think the template is setting any module names...?

blueraft commented 3 months ago

The template asks for two names - plugin_name and module_name.

The plugin_name is also set as the project_name in the pyproject.toml - this follows the convention of using hyphens in project names.

But the python package is located in src/module_name which is the name that has to have underscores.

The two can be different but what I've noticed is that, if it's different sometimes setuptools doesn't build the package with the right metadata. So the importlib.metadata functions fail. Sometimes it works though 🤷‍♂️

lauri-codes commented 3 months ago

I see. A few notes:

Examples:

plugin_name given in template pip install name / Git repo name package name in Python imports
test nomad-test nomad_test
my_plugin nomad-my-plugin nomad_my_plugin
my-plugin nomad-my-plugin nomad_my_plugin

To me the template should not be made overly complicated. If the user wants some kind of extra special naming or multiple top-level packages to be installed from a single pyproject.toml, I think they should do the change manually.

blueraft commented 3 months ago

is there a point in the template asking for these two things separately?

Not necessarily. This template was forked from the pytest template, and there they had the used the same module_name, plugin_name questions. We do need the variable module_name or equivalent through out the project though. There's a way to inject extra context variables without asking the user but I've not been able to get it to work yet.

I'll keep this issue open until we find a satisfactory solution to this but in the mean time I'll update the question to let the user know that they don't have to answer it and just press continue.