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

Added general PPMS plugin #133

Merged
merged 30 commits into from
Dec 19, 2024
Merged

Conversation

JonathanNoky
Copy link
Collaborator

Added the PPMS plugin. Up to now, it can deal with ETO and ACT measurements. More types like magnetization measurement will follow soon.

@JonathanNoky JonathanNoky linked an issue Oct 10, 2024 that may be closed by this pull request
@aalbino2
Copy link
Contributor

FYI @budschi

@aalbino2 aalbino2 self-requested a review December 16, 2024 15:35
@aalbino2
Copy link
Contributor

aalbino2 commented Dec 16, 2024

hi @JonathanNoky

few points:

  • you find now def set_entrydata_definition(self): around in the code. Here is where you can add any kind of class that must be used by the parser. In this way, you can inherit and specialize this parser into your plugin, you can change the classes used, and also extend the set of variable defined to include new classes
  • if you use the virtualenvironment where nomad is installed you get also the ruff linting tools (ruff check . --fix and `ruff format .) so you get linted code
  • more comments in specific lines of the MR
  • insert test data in the folder tests/data/ppms so we can test the plugin

hampusnasstrom and others added 4 commits December 16, 2024 16:45
Updated `merge_sections` util function to work with the latest nomad-lab version
* Added breaking test for boolean array

* Added more breaking test cases

* Added fix for comparing non numpy arrays

* Ruff

* Enhance test_merge_sections to capture output and validate float_array values

* Refine warning message for merging sections with differing quantity values

* Refactor merge_sections to improve warning logic for differing quantity values
* Added breaking test for float array with units

* Added fix for comparison of pint quantity arrays

* Added test for multi dimensional array

* Ruff
* Add fixture to capture error from logs

* Add nomad-lab infrastructure deps required by caplog fixture

* Ruff

* Refactoring fixtures; parameterize caplog to allow capturing different log levels

* Removing the formatter from nomad.utils

* testing: add logger.error in normalize

* testing: adding the formatter from nomad.utils

* Add logstash dep; cleaning

* Cleaning

* Specify caplog as an arg (from review)
@aalbino2
Copy link
Contributor

Hey @ka-sarthak, any hint why this is failing?

https://github.com/FAIRmat-NFDI/nomad-measurements/actions/runs/12356184054/job/34481491564?pr=133

it says it doesn't find mongoengine

@ka-sarthak
Copy link
Collaborator

ka-sarthak commented Dec 16, 2024

It seems to be coming when using nomad.search in the ppms parser. This is strange, as we have always used nomad.search, however in the normalize and not the parser, without having to install mongoengine

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/opt/hostedtoolcache/Python/3.11.11/x64/lib/python3.11/site-packages/nomad/client/processing.py:46: in parse
    from nomad.parsing import parsers
/opt/hostedtoolcache/Python/3.11.11/x64/lib/python3.11/site-packages/nomad/parsing/parsers.py:248: in <module>
    instance = entry_point.load()
src/nomad_measurements/ppms/__init__.py:10: in load
    from nomad_measurements.ppms.parser import PPMSParser
src/nomad_measurements/ppms/parser.py:33: in <module>
    from nomad.search import search
/opt/hostedtoolcache/Python/3.11.11/x64/lib/python3.11/site-packages/nomad/search.py:59: in <module>
    from nomad import datamodel, infrastructure, utils
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

    #
    # Copyright The NOMAD Authors.
    #
    # This file is part of NOMAD. See https://nomad-lab.eu/ for further info.
    #
    # Licensed under the Apache License, Version 2.0 (the "License");
    # you may not use this file except in compliance with the License.
    # You may obtain a copy of the License at
    #
    #     http://www.apache.org/licenses/LICENSE-2.0
    #
    # Unless required by applicable law or agreed to in writing, software
    # distributed under the License is distributed on an "AS IS" BASIS,
    # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    # See the License for the specific language governing permissions and
    # limitations under the License.
    #
    
    """
    This module provides function to establish connections to the database, searchengine, etc.
    infrastructure services. Usually everything is setup at once with :func:`setup`. This
    is run once for each *api* and *worker* process. Individual functions for partial setups
    exist to facilitate testing, aspects of :py:mod:`nomad.cli`, etc.
    """
    
    import os.path
    import os
    import shutil
    from elasticsearch_dsl import connections
>   from mongoengine import connect, disconnect
E   ModuleNotFoundError: No module named 'mongoengine'

src/nomad_measurements/ppms/parser.py Outdated Show resolved Hide resolved
src/nomad_measurements/ppms/__init__.py Outdated Show resolved Hide resolved
Copy link
Contributor

@aalbino2 aalbino2 left a comment

Choose a reason for hiding this comment

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

@JonathanNoky tell me if some point is not clear

@aalbino2
Copy link
Contributor

If you see in this branch, I moved the test file for XRD in a dedicated folder. I will need to change the paths in the test.

IS it okay for you to organize the test files in subfolders @ka-sarthak ?

@ka-sarthak
Copy link
Collaborator

@aalbino2 Sure, tests/data/xrd looks good

@aalbino2 aalbino2 mentioned this pull request Dec 17, 2024
Copy link
Contributor

@aalbino2 aalbino2 left a comment

Choose a reason for hiding this comment

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

The sequence file leads to a failure.

Probably the way we pass the class to be instantiated must be modified:

The other .dat file is not matched.

@JonathanNoky

Added call of set_entrydata_definition to parse
@aalbino2 aalbino2 linked an issue Dec 17, 2024 that may be closed by this pull request
@aalbino2
Copy link
Contributor

I will merge this PR and open new issues for further development @JonathanNoky

@aalbino2 aalbino2 merged commit 81e3c1d into main Dec 19, 2024
5 checks passed
@aalbino2 aalbino2 deleted the 132-move-the-ppms-parser-from-area-a-repo branch December 19, 2024 15:21
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.

PPMS tests Move the PPMS parser from Area A repo
4 participants