Add NMR data model, integration with EnzymeML, and peak assignment features

[torogi94]

• Add md-models-based research data model for NMR
  • Collect NMR parameters from instrument metadata
  • Record processing steps preformed in NMRpy
  • Save both the raw data and the latest version of the processed data
  • Assign species from a list or from an EnzymeML document to the peaks
• Add EnzymeML integration using pyenzyme library
  • Load EnzymeML document into a FidArray
  • Have EnzymeML species available within Fid object
  • Save calculated concentrations to and return the EnzymeML document
• Add widgets to assign peaks either on the entire FidArray or single Fid objects
  • Use EnzymeML species if document available
  • Pass list of EnzymeML species if no document has been loaded yet
  • Use list of simple strings instead of working with EnzymeML standard
  • Use only a slice of Fids when assigning on the entire FidArray by passing an index list

[jmrohwer]

@torogi94 Thanks for this!
I need some time to look through this. But from a brief inspection the following come to mind:

• Dependencies: utils.py imports sympy as well as pyenzyme, yet none of them are declared in requirements.txt. We should discuss dependencies and which of these are needed for core functionality. One option is to have optional dependencies for certain functionalities. Pyenzyme itself has a multitude of dependencies. I am not keen for the core NMRPy library to have a huge list of dependences. Associated with that the version requirement dependency hell.
• Related: how stable are pyenzyme, md-models and the other dependencies from your group? E.g. Pyenzyme vs. Pyenzyme2? My concern is long-term maintenance.
• The merge conflict needs to be resolved.
• Looking at data_objects.py as well as plottling.py there are a very large number of changes relating to linting and formatting (e.g. single vs. double quotes, linebreaks etc.). Are these linting changes done in a separate commit? As it is, it's very tedious to separate the real code changes from "cosmetic" linting.

[torogi94]

Thank you for looking over it! Here, a few comments:

Dependencies: utils.py imports sympy as well as pyenzyme, yet none of them are declared in requirements.txt. We should discuss dependencies and which of these are needed for core functionality. One option is to have optional dependencies for certain functionalities. Pyenzyme itself has a multitude of dependencies. I am not keen for the core NMRPy library to have a huge list of dependences. Associated with that the version requirement dependency hell.

Good catch! I really did forget to declare the new dependencies in the requirements.txt file. I think you are right that we should not necessarily introduce them to the core of NMRpy. How about we declare them as optional nmrpy[enzymeml]? For the NMRpy data model, however, it would be easier if we could make pydantic a core dependency. If I am not mistaken, it is the only one required for the data model to work. If not, I can restructure my code, so the data model part becomes entirely optional, too, though.

Related: how stable are pyenzyme, md-models and the other dependencies from your group? E.g. Pyenzyme vs. Pyenzyme2? My concern is long-term maintenance.

We are preparing a new stable pyenzyme (v2) release which we can then pin in the requirements. In my experience, it is already running better and will be more maintainable than the previous v1 version. With regard to md-models, the NMRpy data model is not dependent at all on the md-models version. Actually, it is not even a dependency needed, as all functionalities are handled via pydantic, which is itself a very well maintained library!

The merge conflict needs to be resolved.

Absolutely. I’ll merge main into my branch and fix the conflict manually so you can have a clean merge without conflicts!

Looking at data_objects.py as well as plottling.py there are a very large number of changes relating to linting and formatting (e.g. single vs. double quotes, linebreaks etc.). Are these linting changes done in a separate commit? As it is, it's very tedious to separate the real code changes from "cosmetic" linting.

Sorry for the noisy diff, it seems I accidentally let my linter loose on the two files and did not even notice... I’ll revert the linting changes from this PR and submit them in a separate PR (like discussed).

[jmrohwer]

Good catch! I really did forget to declare the new dependencies in the requirements.txt file. I think you are right that we should not necessarily introduce them to the core of NMRpy. How about we declare them as optional nmrpy[enzymeml]? For the NMRpy data model, however, it would be easier if we could make pydantic a core dependency. If I am not mistaken, it is the only one required for the data model to work. If not, I can restructure my code, so the data model part becomes entirely optional, too, though.

I agree with making pydantic a core dependency and the others as optional in nmrpy[enzymeml].

On another note. I also have a conda build recipe as well as a CI job that builds and distributes a conda package. This has been very easy to date because NMRPy is a pure python package and there are/were conda packages available for all of the dependencies (channel conda-forge). Is PyEnzyme released as a conda package? I would like to continue distributing conda packages, just as we do for PySCeS as well.

We are preparing a new stable pyenzyme (v2) release which we can then pin in the requirements. In my experience, it is already running better and will be more maintainable than the previous v1 version. With regard to md-models, the NMRpy data model is not dependent at all on the md-models version. Actually, it is not even a dependency needed, as all functionalities are handled via pydantic, which is itself a very well maintained library!

Okay great.

Sorry for the noisy diff, it seems I accidentally let my linter loose on the two files and did not even notice... I’ll revert the linting changes from this PR and submit them in a separate PR (like discussed).

Give me a ping once you are ready with all of this then I'll proceed with the detailed code review 😄

[torogi94]

I think I have addressed all points mentioned so far, so you may continue with the detailed code review now!

On another note. I also have a conda build recipe as well as a CI job that builds and distributes a conda package. This has been very easy to date because NMRPy is a pure python package and there are/were conda packages available for all of the dependencies (channel conda-forge). Is PyEnzyme released as a conda package? I would like to continue distributing conda packages, just as we do for PySCeS as well.

I am in contact with @JR-1991 about the conda-forge release. While it was not planned initially, he says we can make it happen! I will come back to you about this when we are preparing the PyPI release of v2. For now, the GitHub version is still used in the new optional requirements for nmrpy[enzymeml]. Of course, I will change that to the stable PyPI version as soon as it is available!

[torogi94]

I have addressed a few more issues we discussed in pull requests #14 and #15 and merged them with this pull request:

• With Add measurement handling #14, handling of multiple EnzymeML measurements is now possible.
• With Rework data handling #15, data handling within NMRpy has been optimized to keep memory requirements and computation times low.

[torogi94]

@jmrohwer: In #18, I have added handling of t0 values in MeasurementData of EnzymeML documents to the library. Initial values can now be set using FidArray.add_t0_to_enzymeml(). Both an option for interactive assignment using the new T0Adder widget and script-like handling when setting gui=False are available. The user has the option to either using the existing t1 values as t0, if no initials have been measured, or provide measured t0 values instead. Furthermore, an offset value to account for delays in measurements can be applied to the time array.

[Copilot]

Pull request overview

This PR introduces an NMR-focused research data model aligned with md-models/EnzymeML, adds EnzymeML integration (via optional pyenzyme), and extends interactive widgets for peak assignment and measurement editing.

Changes:

• Added NMRpy/EnzymeML-aligned data model specifications and an auto-generated Pydantic schema (JSON-LD capable).
• Implemented EnzymeML helpers (species lookup, measurement construction, applying concentrations) and new peak/t0/measurement widgets.
• Updated packaging/tooling (optional extras for EnzymeML, dependency bumps, Ruff config) and added/extended tests.

Reviewed changes

Copilot reviewed 9 out of 10 changed files in this pull request and generated 18 comments.

Show a summary per file

File	Description
specifications/nmrpy.md	Adds written spec for the NMRpy data model objects/enums.
specifications/datamodel_schema.md	Adds Mermaid class diagram for the data model structure.
setup.py	Adds EnzymeML optional extras and updates author metadata.
ruff.toml	Introduces Ruff formatting/lint configuration.
requirements.txt	Updates baseline dependencies and adds pydantic.
nmrpy/utils.py	Adds EnzymeML utility helpers (species, measurements, serialization helpers).
nmrpy/tests/nmrpy_tests.py	Adds tests for new data model + EnzymeML-related setters.
nmrpy/plotting.py	Adds peak assignment + t0/measurement widgets and integrates EnzymeML utilities.
nmrpy/nmrpy_model.py	Adds generated Pydantic models for the NMRpy data model (JSON-LD fields).
nmrpy/data_objects.py	Integrates data model + EnzymeML into Fid/FidArray and processing steps tracking.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This unconditional pyenzyme import will raise at import-time when pyenzyme isn't installed, even though the rest of the file treats it as an optional dependency. Move Protein into the existing try: block (or avoid importing it directly and use getattr(pyenzyme, ...) / isinstance(..., pyenzyme.Protein) inside the guarded section).

[jmrohwer]

@torogi94 Please look into this and confirm that you agree. Only usage of Protein is in line 1758. How will this affect the functionality if pyenzyme is not installed? Does the PeakAssigner class depend on pyenzyme? I'm assuming so, see

nmrpy/nmrpy/data_objects.py

Lines 1467 to 1475 in 1928d7c

	if (pyenzyme is None) and (isinstance(species_list, EnzymeMLDocument)):
	raise RuntimeError(
	'The `pyenzyme` package is required to use NMRpy with an EnzymeML document. Please install it via `pip install nmrpy[enzymeml]`.'
	)
	self._assigner_widget = PeakAssigner(
	fid=self,
	species_list=species_list,
	title='Assign species for {}'.format(self.id),
	)

Thus seems okay to move the Protein import into the existing try/except block. Done.

[Copilot]

float(datum) will raise TypeError when self.data contains complex values (common before .real()), but only AttributeError is caught here. Update serialization logic to handle complex processed data (e.g., store strings for complex values) and/or catch TypeError to avoid hard crashes when accessing fid_object.

Suggested change

	self.__fid_object.raw_data = [str(datum) for datum in self.raw_data]
	self.__fid_object.processed_data = [float(datum) for datum in self.data]
	except AttributeError:
	# Always serialise raw_data as strings
	self.__fid_object.raw_data = [str(datum) for datum in self.raw_data]
	# Serialise processed_data, handling complex or otherwise non-floatable values gracefully
	processed_data = []
	for datum in self.data:
	try:
	processed_data.append(float(datum))
	except (TypeError, ValueError):
	# Fallback for complex or non-scalar values: store string representation
	processed_data.append(str(datum))
	self.__fid_object.processed_data = processed_data
	except (AttributeError, TypeError):

[jmrohwer]

Calling float(datum) on a complex128 will not raise a TypeError but just issue a ComplexWarning and discard the imaginary part. Are we going to just ignore this error message? The point is, do you want the FIDObject to handle complex data in the processed_data list? This is an object that got introduced with the data model.

[jmrohwer]

@torogi94 Before merge, we actually need to deal with the Copilot comments. I've been through the code and they have not yet been applied. Most of them seem useful and true, and indeed correct errors, so I'd appreciate it if you could go through them yourself as well. The issue of p0/p1 in radians vs. degrees I need to investigate a bit further myself but don't have the time right now.

[jmrohwer]

@torogi94 I have dealt with most of the remaining issues from the Copilot review, but there are some outstanding ones that you need to deal with - I have mentioned you in the comments. Please review the changes in the last commit (79f1ff5) if you agree.

Also FYI I updated the build system to use pyproject.toml and get rid of the deprecated setup.py. Been meaning to do this for a while.

[jmrohwer]

3 more commits to fix compatibility bugs. Numpy 2.5 has many deprecations, so the nmrglue import crashes out with a bug. There has not been a new release yet (the last one was 2 years ago!) so for the time being we have to get the requirement from github.

Furthermore another numpy 2.4 (which numpy are you using @torogi94 ?) deprecation caused one of the tests to fail. Fixed.

Also fixed another bug that manifested if pyenzyme was not installed.

Checked that all tests (except plotting) now pass when pyenzyme is installed and when not. In a new environment with the latest version of all packages.

[jmrohwer]

Updated the scripts for Anaconda build and for CI. The tests are split out now that first the tests run without pyenzyme installed and then the remaining ones after installing pyenzyme. All passes if you look at the CI runs so it should be good to go 🤔
Except for the Copilot reviews that still need resolving as discussed.