Testing pipeline

[JosePizarro3]

I am testing the pipeline and restructuring a bit the project to look more aligned with the central NOMAD and other plugins which are integrated in the software.

[coveralls]

Pull Request Test Coverage Report for Build 8285970780

Details

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage remained the same at 84.375%

---

Totals
Change from base Build 8283702209:	0.0%
Covered Lines:	27
Relevant Lines:	32

---

💛 - Coveralls

[JosePizarro3]

@ladinesa @ka-sarthak any idea why the pipeline does not work? I just renamed src to analysis and moved the modules one level up (before they were under src/nomad_analysis.

Everything else, I tried to copy from the example, so I am really puzzled.

Btw, as an additional question: do we need to test parsing from nomad.cli? I think we can get rid off these in this repo anyways.

[ka-sarthak]

Hi @JosePizarro3

I had similar test failures earlier and it was probably because the nomad parse was unable to find the analysis schema. If you export the path of the new src (or analysis in this case) to PYTHONPATH, parse function might work. A better solution is to add the name of folder containing the package code to this quantity in pyproject.toml and then re-build the package.

[tool.setuptools.packages.find]
where = ["src"]

But the choice for having src/nomad-analysis was a deliberate one. It's a good practice to have src folder that contains the package code (read more here). I also raised an issue to support this in nomad-schema-plugin-example of which this repo is a fork. nomad-coe/nomad-schema-plugin-example#9

Similar structure is being used for nomad-measurements, nomad-material-properties, and other plugins being developed in Area A.

[JosePizarro3]

Hi @JosePizarro3

I had similar test failures earlier and it was probably because the nomad parse was unable to find the analysis schema. If you export the path of the new src (or analysis in this case) to PYTHONPATH, parse function might work. A better solution is to add the name of folder containing the package code to this quantity in pyproject.toml and then re-build the package.

[tool.setuptools.packages.find]
where = ["src"]

But the choice for having src/nomad-analysis was a deliberate one. It's a good practice to have src folder that contains the package code (read more here). I also raised an issue to support this in nomad-schema-plugin-example of which this repo is a fork. nomad-coe#9

Similar structure is being used for nomad-measurements, nomad-material-properties, and other plugins being developed in Area A.

Yeah, thanks for the info. My question is why renaming does not work, and the plugin example works with a flat folder structure (and all the other plugins we have ...).

Personally I am not friend of overnesting stuff (we are essentially having 2 folders instead of a flat hierarchy. But I am happy with whatever. We just need to be consistent.

[JosePizarro3]

One remark on the structure: I think nomad-analysis could be itself a placeholder for individual plugins which then get hooked up; something like:

.
└── nomad-analysis/
    └── src/
        ├── spectral_profiles_analysis/
        │   ├── nomad_plugin.yaml
        │   └── ...
        ├── descriptors_analysis/
        │   ├── nomad_plugin.yaml
        │   └── ...
        └── <whatever>_analysis/
            ├── nomad_plugin.yaml
            └── ...

Still, I am puzzled by the fact that the pipeline mimicking the nomad-parser-plugin-example does not work. I will keep investigating tomorrow.

[ka-sarthak]

Hi @JosePizarro3, I tested the branch in my local and the pytest runs successfully. Even the one using coverage which seems to be failing the action in the workflow.

python -m coverage run -m pytest -sv

I also added some comments on regarding the changes you made. Please review them :)

Regarding the file structure rearrangement, I will still argue in favor of the src-layout, as it makes the testing of the installed package more robust. Also, I think we should have a nomad_analysis module within src to allows for much nicer import statement.

.
└── src/
    └── nomad_analysis/
        ├── spectral_profiles/
        │   ├── nomad_plugin.yaml
        │   └── ...
        ├── descriptors/
        │   ├── nomad_plugin.yaml
        │   └── ...
        └── <whatever>/
            ├── nomad_plugin.yaml
            └── ...

As the installed package will use everything inside /src, the import statements for the installed package would look like:

from nomad_analysis.spectral_profiles import some_class

I think this is more descriptive and navigable than a generic analysis.

[aalbino2]

@JosePizarro3 don't screw up the plugin structure we have here, we can have a meeting and dicsuss why it is like that before you change it

[JosePizarro3]

@JosePizarro3 don't screw up the plugin structure we have here, we can have a meeting and dicsuss why it is like that before you change it

Hey, this is just a draft where I didn't want any review yet, nor comments except for my questions.

I wanted to play with the pipeline, because the parser-plugin-example has a very different layout. I want to learn and standarize the way of developing and distributing plugins, so like I said to @ka-sarthak (in private), I am totally ok for using this layout, and adapt this to any other plugin.

[aalbino2]

@JosePizarro3 don't screw up the plugin structure we have here, we can have a meeting and dicsuss why it is like that before you change it

Hey, this is just a draft where I didn't want any review yet, nor comments except for my questions.

I wanted to play with the pipeline, because the parser-plugin-example has a very different layout. I want to learn and standarize the way of developing and distributing plugins, so like I said to @ka-sarthak (in private), I am totally ok for using this layout, and adapt this to any other plugin.

Oki 😇 Just thought you wanted to change it quick. Anyway, as Sarthak may have explained already, it's mainly for properly pip installability of the plugin, I also didn't enjoy much nesting, but then I found is more convenient when using it

Looking forward to see how this plugin evolves

[JosePizarro3]

Regarding the file structure rearrangement, I will still argue in favor of the src-layout, as it makes the testing of the installed package more robust. Also, I think we should have a nomad_analysis module within src to allows for much nicer import statement.

.
└── src/
    └── nomad_analysis/
        ├── spectral_profiles/
        │   ├── nomad_plugin.yaml
        │   └── ...
        ├── descriptors/
        │   ├── nomad_plugin.yaml
        │   └── ...
        └── <whatever>/
            ├── nomad_plugin.yaml
            └── ...

As the installed package will use everything inside /src, the import statements for the installed package would look like:

from nomad_analysis.spectral_profiles import some_class

I think this is more descriptive and navigable than a generic analysis.

Yeah, let's keep it src/nomad_analysis. As for now we only have spectra analysis, we keep it without further folder structure, and later we can move modules around.

Ok, I will restore the organization, setup.py, and just change a bit the code. Then it will be ready for review 😛

[hampusnasstrom]

I was listed under "reviewers" so I guess that meant you wanted me to review this @ka-sarthak? Added some comments and suggestions.

[JosePizarro3]

@ka-sarthak this is ready I think.

Please, note the Ruff formatting for your branches in the future 🙂 The pipeline now works as it should regarding ruff format . --check.

[ka-sarthak]

Looks great to me!! 😄

I have added some comments on the README containing some minor fixes and rewriting suggestion. Also, there's an import statement in the test_schema.py that can be removed.

I am approving it, but please make sure to integrate the review before merging.

I also think you will have to rebase to the current develop before merging.