Fix issue 292: amending for mass spectrometry specific objects, properties and controlled vocabularies by janlisec · Pull Request #302 · BAMresearch/bam-masterdata

janlisec · 2026-05-28T16:29:06Z

existing object types amended by new properties:

INSTRUMENT.MASS.SPEC
EXPERIMENTAL_STEP.MS_BATCH
new object types:
INSTRUMENT.LC_SYSTEM
INSTRUMENT.GC_SYSTEM
several new contr.vocab
!! I removed (outcommented) one existing property (MS_IONIZATION_MODE) to avoid redundancy with a novel property MS_ION_POLARITY -- please check if this causes inconsistencies. If so I would modify the PR and use the old property.

@janlisec

…anges). Next step is revision by @janlisec.

JosePizarro3

Hi, thanks for the work, I have some minor comments + 2 larger ones:

Maybe it is a good idea to clarify the role of CHROMOGRAPHY_TYPE and the two objects (LC_SYSTEM, GC_SYSTEM) you want to define. It seems this could be better represented by a property of type OBJECT and a common object type called CHROMATOGRAPHY.
Would it be a good idea to move MS-specific definitions to its own subfolder, .../datamodel/mass_spectrometry? Similar to what we have in welding/.

JosePizarro3 · 2026-05-28T20:24:35Z

-        section="MS Information",
-    )
+    # changes
+    # I would remove this property, as it is redundant with MS_ION_POLARITY. However should it be in use already, I would keep this one and not establish MS_ION_POLARITY.


I will check this

JosePizarro3 · 2026-05-28T20:25:05Z

        code="MS_HYPHENATION_METHOD",
        data_type="VARCHAR",
        property_label="Hyphenation method",
        description="""Hyphenation (DI, LC, GC, CE)//Probeninjektion (DI, LC, GC, CE)""",


Are these the only options or can there be more?

We can consider removing this property or (better) subsituting against the new property CHROMATOGRAPHY_TYPE. MS_HYPHENATION_METHOD is basically redundant with CHROMATOGRAPHY_TYPE where the latter contains contr.vocab.

JosePizarro3 · 2026-05-28T20:25:51Z

+    ms_intensity_range = PropertyTypeAssignment(
+        code="MS_INTENSITY_RANGE",
+        data_type="VARCHAR",
+        property_label="Intensity range",


if this is a range, I would like to see something else than a VARCHAR, perhaps a min/max couple of REAL properties (and the units)

I think we could avoid this formal strictness here, but I can make the changes if you like.

JosePizarro3 · 2026-05-28T20:26:27Z

+        code="MS_RESOLUTION",
+        data_type="REAL",
+        property_label="Resolution",
+        description="""Approximate Resolution//Ungefähre Auflösung""",


Do you mind elaborating? is this some sort of pixel or m/z?

Resolution {numeric, no unit} = m2 / (m2-m1) = value indicating if the peaks of two m/z measured at the same time can be clearly differentiated. We measure ion intensities (y) on a continuous scale for m/z (x). While mass precision is a parameter for the peak apex (is it at 123.4 or at 123.45), resolution is a parameter for peak width. Best example: consider compound CN. m would be 12.000 + 14.003 = 26.003. It has two isotopes, [13]CN = 13.007 + 14.003 = 27.010 and C[15]N = 12.000 + 15.000 = 27.000. We could measure both isotopic masses with sufficient precision independently but without sufficient resolution we would measure a single peak instead of two. 'approximate' is in there, because resolution is dependent on device type and mass, i.e. TOF-MS devices would have lower resolution for lower masses, Orbitrap devices would have higher resolution for lower masses.

JosePizarro3 · 2026-05-28T20:26:55Z

+        section="MS Parameters",
+    )
+
+    ms_scan_rate = PropertyTypeAssignment(


yeah, I am a bit confused by this and the ms_resolution

MS_SCAN_RATE is the time resolution of the device, either specified Hz [1/s] or sometimes in milli seconds per scan [ms].

JosePizarro3 · 2026-05-28T20:31:08Z

+    )
+
+
+class GcSystem(Instrument):


Mmm, for now, LcSystem and GcSystem contain the same properties, so I'd argue that both can be simply defined as a single entity and then add something like kind = ["LC", "GC"] to differentiate between them.

It is even related with CHROMATOGRAPHY_TYPE in the MassSpec object.

Or are you thinking of other potential properties that might be defined later on?

I had long discussions with Caro on how to represent our intruments best. I would keep both systems apart because more system-type specific properties might be added in the future. Actually, we run these machines in combinations GC-MS or LC-MS, combining a separation device with one or several detector devices. I had originally specified INSTRUMENT.ANALYTICAL_SYSTEM to reflect this, but Caro suggested otherwise, stating that this was too broad.

JosePizarro3 · 2026-05-28T20:31:23Z

 from bam_masterdata.metadata.definitions import VocabularyTerm, VocabularyTypeDef
 from bam_masterdata.metadata.entities import VocabularyType

+# changes


you can delete these

JosePizarro3 · 2026-05-28T20:32:22Z

+        section="Technical Details",
+    )
+
+    chromatography = PropertyTypeAssignment(


if you defined LcSystem and GcSystem I wondered whether this is necessary or, better, whether what you need is a property of type OBJECT

JosePizarro3 · 2026-05-28T20:33:13Z

+        description="""Ion Chromatography//Ionenchromatographie""",
+    )
+
+    none = VocabularyTerm(


no need of NONE, this can be assigned when the property is not mandatory and nothing is stored

Please keep it. It is good to be expicit here and allow to state that no separation method was used before injection to the MS.

JosePizarro3 · 2026-05-28T20:33:46Z

+        description="""Ion polarity//Ionenpolarität""",
+    )
+
+    pos = VocabularyTerm(


more verbose please, POSITIVE, NEGATIVE, POSITIVE_NEGATIVE

janlisec · 2026-06-03T12:27:48Z

Hi, thanks for the work, I have some minor comments + 2 larger ones:

Maybe it is a good idea to clarify the role of CHROMOGRAPHY_TYPE and the two objects (LC_SYSTEM, GC_SYSTEM) you want to define. It seems this could be better represented by a property of type OBJECT and a common object type called CHROMATOGRAPHY.

Would it be a good idea to move MS-specific definitions to its own subfolder, .../datamodel/mass_spectrometry? Similar to what we have in welding/.

[1] I would like to upload the device objects of our group so that users can link there measurements to these devices and other groups can see what we have got. We had originally a very broad INSTRUMENT.ANALYTICAL_SYSTEM, got more specific now and also could set up INSTURMENT.CHROMATOGRAPHY as an intermediate solution (but would potentially blow this up with type specific properties in the long run). All solutions have advantages and disadvantages... :/

[2] fine with me, especially when we can expect more specific property sets to appear in the future

--> once you made decisions on the above issues, I would modify the PR accordingly, correct?

caro-demi and others added 2 commits May 21, 2026 14:24

First changes are added to object_pytes and vocabulary_types (see #ch…

94451a9

…anges). Next step is revision by @janlisec.

new definitions checked and confirmed, added ContrVocab

003da4e

janlisec requested a review from JosePizarro3 as a code owner May 28, 2026 16:29

janlisec added 3 commits May 28, 2026 18:43

correcting errors reported by CI on PR

3cd7ffd

fix CI error on improper CODE term 'MS/MS'

830ea81

fix CI error on another CODE

ad5c054

JosePizarro3 added the datamodel Changes in the datamodel label May 28, 2026

JosePizarro3 reviewed May 28, 2026

View reviewed changes

		)


		class GcSystem(Instrument):

Conversation

janlisec commented May 28, 2026

Uh oh!

JosePizarro3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

janlisec commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

janlisec commented Jun 3, 2026 •

edited

Loading