Skip to content
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

Move NOMAD examples to plugin #448

Merged
merged 48 commits into from
Nov 29, 2024
Merged

Conversation

lukaspie
Copy link
Collaborator

@lukaspie lukaspie commented Oct 9, 2024

  • build entrypoints
  • include in MANIFEST.in
  • include in pyproject.toml
  • check if all plugins are really up-to-date -> there may be updates in other NOMAD branches that are not included here.
  • decide whether some of the examples or at least some of the data shall be downloaded instead of stored in Git
  • can we have a mechanism so that the pynxtools plugins have their own nomad examples and these are brought into the central pynxtools only if the plugin is installed? This will likely be tricky
    EDIT: we can just have the data and the nomad plugin entrypoint within the plugins themselves
  • test for example data and ExampleUploadEntryPoint
  • test within NOMAD

@lukaspie lukaspie marked this pull request as draft October 9, 2024 11:10
@rettigl
Copy link
Collaborator

rettigl commented Oct 9, 2024

  • can we have a mechanism so that the pynxtools plugins have their own nomad examples and these are brought into the central pynxtools only if the plugin is installed? This will likely be trickier than

Such a solution would from my point of view be the most logical place to locate these examples, i.e. together with each reader. In any case, we should follow a SSOT approach for these examples.

@lukaspie
Copy link
Collaborator Author

lukaspie commented Oct 9, 2024

  • can we have a mechanism so that the pynxtools plugins have their own nomad examples and these are brought into the central pynxtools only if the plugin is installed? This will likely be trickier than

Such a solution would from my point of view be the most logical place to locate these examples, i.e. together with each reader. In any case, we should follow a SSOT approach for these examples.

Absolutely, because as of now, we not only need to update them in NORTH and in nomad-FAIR, but also in all distribution branches in nomad-distro. Can we not try to download them from NORTH? We need them there anyway because the CI/CD checks that the examples work with the installed image.

@rettigl
Copy link
Collaborator

rettigl commented Oct 9, 2024

  • can we have a mechanism so that the pynxtools plugins have their own nomad examples and these are brought into the central pynxtools only if the plugin is installed? This will likely be trickier than

Such a solution would from my point of view be the most logical place to locate these examples, i.e. together with each reader. In any case, we should follow a SSOT approach for these examples.

Absolutely, because as of now, we not only need to update them in NORTH and in nomad-FAIR, but also in all distribution branches in nomad-distro. Can we not try to download them from NORTH? We need them there anyway because the CI/CD checks that the examples work with the installed image.

It feels a bit strange having them in NORTH at all, because there is not a 1:1 mapping of NORTH tools and pynxtools plugins, i.e. there is not a tool for every example/plugin, no? Maybe one could create another example repo with sub-repos for each example, which then gets submoduled where needed? However, then one still needs to update the submodule commits all the time, so also not ideal.

@lukaspie
Copy link
Collaborator Author

lukaspie commented Oct 9, 2024

  • can we have a mechanism so that the pynxtools plugins have their own nomad examples and these are brought into the central pynxtools only if the plugin is installed? This will likely be trickier than

Such a solution would from my point of view be the most logical place to locate these examples, i.e. together with each reader. In any case, we should follow a SSOT approach for these examples.

Absolutely, because as of now, we not only need to update them in NORTH and in nomad-FAIR, but also in all distribution branches in nomad-distro. Can we not try to download them from NORTH? We need them there anyway because the CI/CD checks that the examples work with the installed image.

It feels a bit strange having them in NORTH at all, because there is not a 1:1 mapping of NORTH tools and pynxtools plugins, i.e. there is not a tool for every example/plugin, no? Maybe one could create another example repo with sub-repos for each example, which then gets submoduled where needed? However, then one still needs to update the submodule commits all the time, so also not ideal.

I think what we could do is have the pynxtools plugins be the SSOT place that have these folders with the example files that shall be used in NOMAD. Then instead of having the files also in NORTH, we can just get them from GitHub in the NORTH CI/CD for testing. And for the NORTH tools that don't have a directly correspondant plugin, we just don't use this.

Then for the NOMAD examples, we use the url option in ExampleUploadEntryPoint (see docs) and point this to the URL of the examples in the repo.

@lukaspie
Copy link
Collaborator Author

lukaspie commented Oct 9, 2024

  • can we have a mechanism so that the pynxtools plugins have their own nomad examples and these are brought into the central pynxtools only if the plugin is installed? This will likely be trickier than

Such a solution would from my point of view be the most logical place to locate these examples, i.e. together with each reader. In any case, we should follow a SSOT approach for these examples.

Absolutely, because as of now, we not only need to update them in NORTH and in nomad-FAIR, but also in all distribution branches in nomad-distro. Can we not try to download them from NORTH? We need them there anyway because the CI/CD checks that the examples work with the installed image.

It feels a bit strange having them in NORTH at all, because there is not a 1:1 mapping of NORTH tools and pynxtools plugins, i.e. there is not a tool for every example/plugin, no? Maybe one could create another example repo with sub-repos for each example, which then gets submoduled where needed? However, then one still needs to update the submodule commits all the time, so also not ideal.

I think what we could do is have the pynxtools plugins be the SSOT place that have these folders with the example files that shall be used in NOMAD. Then instead of having the files also in NORTH, we can just get them from GitHub in the NORTH CI/CD for testing. And for the NORTH tools that don't have a directly correspondant plugin, we just don't use this.

Then for the NOMAD examples, we use the url option in ExampleUploadEntryPoint (see docs) and point this to the URL of the examples in the repo.

I still think we need to define the ExampleUploadEntryPoint instances directly in pynxtools and cannot delegate that to the plugins because you need to declare the nomad plugin entry-points in the pyproject.toml of the nomad plugin itself, like so:

[project.entry-points.'nomad.plugin']
apm_example = "pynxtools.nomad.entrypoints:apm_example"
ellips_example = "pynxtools.nomad.entrypoints:apm_example"
em_example = "pynxtools.nomad.entrypoints:em_example"

@lukaspie
Copy link
Collaborator Author

lukaspie commented Oct 9, 2024

Then for the NOMAD examples, we use the url option in ExampleUploadEntryPoint (see docs) and point this to the URL of the examples in the repo.

I think we can just use https://download-directory.github.io to directly get the examples from a different repo as a zip file. Or we use git subtree.

@rettigl
Copy link
Collaborator

rettigl commented Oct 10, 2024

This sounds like a reasonable approach. With the requirements for the entry points, I am not sure I understand this well enough

@lukaspie
Copy link
Collaborator Author

What I hadn't figured out: we can just have the entry points for the examples in the individual plugins, see FAIRmat-NFDI/pynxtools-mpes#32. That makes it so much cleaner because the data can just live in the plugin repo and we don't need to use the url for the entry point.

@@ -22,31 +22,31 @@ jobs:
include:

- plugin: pynxtools-ellips
branch: main
branch: bring-in-examples
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reminder to revert

Copy link
Collaborator

@RubelMozumder RubelMozumder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I noticed, you may want to confirm that.

src/pynxtools/nomad/entrypoints.py Outdated Show resolved Hide resolved
src/pynxtools/testing/nomad_example.py Outdated Show resolved Hide resolved
src/pynxtools/testing/nomad_example.py Outdated Show resolved Hide resolved
@lukaspie lukaspie force-pushed the move-nomad-examples-to-plugin branch 2 times, most recently from 42a7bad to c8c6f75 Compare November 1, 2024 16:41
@lukaspie lukaspie force-pushed the move-nomad-examples-to-plugin branch 2 times, most recently from 1efe5c6 to c8177f5 Compare November 27, 2024 00:37
@lukaspie lukaspie marked this pull request as ready for review November 27, 2024 01:04
Copy link
Collaborator

@RubelMozumder RubelMozumder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

src/pynxtools/nomad/entrypoints.py Outdated Show resolved Hide resolved
src/pynxtools/nomad/examples/iv_temp/README.md Outdated Show resolved Hide resolved
src/pynxtools/nomad/examples/iv_temp/README.md Outdated Show resolved Hide resolved
src/pynxtools/nomad/examples/iv_temp/README.md Outdated Show resolved Hide resolved
src/pynxtools/testing/nomad_example.py Show resolved Hide resolved
@lukaspie lukaspie force-pushed the move-nomad-examples-to-plugin branch from 2c3dbc1 to 3959896 Compare November 29, 2024 08:11
@lukaspie lukaspie merged commit cca64f9 into master Nov 29, 2024
16 checks passed
@lukaspie lukaspie deleted the move-nomad-examples-to-plugin branch November 29, 2024 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants