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

Rework NMR instrument manufacturer #12

Closed
8 tasks done
NRayya opened this issue Mar 6, 2024 · 8 comments · Fixed by #40
Closed
8 tasks done

Rework NMR instrument manufacturer #12

NRayya opened this issue Mar 6, 2024 · 8 comments · Fixed by #40
Assignees
Labels
approved Decision regarding this issue was already taken during task force meeting discussion This issue still needs discussion before resolving

Comments

@NRayya
Copy link
Contributor

NRayya commented Mar 6, 2024

  • Import OBI:manufacturer
  • Move NMR instrument manufacturer under OBI:manufacturer and rename to 'NMR device manufacturer'
  • def NMR device manufacturer: A manufacturer that produces NMR instruments and/or their parts.
  • optional - we decided not to do it
    • make ‘NMR instrument manufacturer’ subclass
    • make ‘NMR accessory / consumables manufacturer’ a sibling
    • make ‘historic NMR instrument manufacturer’ subclass
  • make the children all instances (named individuals) instead of classes
    • Some of the latter could then also be asserted to be the same as their OBI equivalents .Bruker is already present as instance in OBI
  • The manufacturers that will be kept in nmrCV are:
    • Agilent Technologies
    • Bruker
    • Doty Scientific
    • JEOL
    • JS Research
    • Kimble Chase
    • MR Resources
    • Oxford Instruments
    • Spinlock SRL
    • Tecmag
    • Varian
    • Wilmad
    • Magritek
    • Nanalysis
    • ThermoFisher
    • NMR Service
    • Deutero
    • Norell
    • Shigemi
    • Chemagnetics
    • Nalorac

We decided that for the time being, we will consider both manufacturers of whole spectrometers and parts as "device manufactures".

  • use OBI:device, which already contains subclasses for parts of NMR instruments (console, magnet) and OBI:NMR instrument under measurement device,
    • hence, move NMR spectrometers under 'OBI:measurement device' and its parts defined in nmrCV under the respective 'OBI:device' class
    • provide a mapping between nmrCV devices and already existing OBI NMR devices in SSSOM format
  • Link the devices parents (e.g., Bruker NMR instrument to the corresponding manufactureres)
@NRayya NRayya added the approved Decision regarding this issue was already taken during task force meeting label Mar 7, 2024
@NRayya
Copy link
Contributor Author

NRayya commented Mar 7, 2024

We decided that for the time being, we will consider both manufacturers of whole spectrometers and parts as "device manufactures".
better to use OBI:device, which already contains subclasses for parts of NMR instruments (console, magnet) and OBI:NMR instrument under measurement device

@StroemPhi StroemPhi self-assigned this Aug 7, 2024
@StroemPhi
Copy link
Contributor

@NRayya should I interpret your last comment that part of resolving this issue is not just reusing OBI:manufacturer and subsume the above listed NMR related ones under there, but also to do the same for the instruments and instrument parts under OBI:device and OBI:NMR instrument repsectively? If yes, we should add this to the description of the issue.

@NRayya
Copy link
Contributor Author

NRayya commented Aug 8, 2024

@NRayya should I interpret your last comment that part of resolving this issue is not just reusing OBI:manufacturer and subsume the above listed NMR related ones under there, but also to do the same for the instruments and instrument parts under OBI:device and OBI:NMR instrument repsectively? If yes, we should add this to the description of the issue.

Done

@StroemPhi
Copy link
Contributor

While working on the implementation in https://github.com/nmrML/nmrCV/tree/12-rework-NMR-instrument-manufacturer I noticed that we have a class 'NMR instrument vendor' which is defined as an 'instrument attribute' (basically a piece of information) that seems redundant to me, since we already have 'NMR device manufacturer'. Can we deprecate this as part of this issue?

@NRayya NRayya added the discussion This issue still needs discussion before resolving label Aug 20, 2024
@StroemPhi
Copy link
Contributor

Working on subsuming existing NMR devices under the respective OBI:device branch entails having to deprecate nmrCV classes that were already defined in OBI (coming originally from the nmrCV crew i.e. Daniel Schober). Now, in the current nmrCV some of these NMR devices are grouped under NMRCV:cardinal part of NMR instrument, but as this class is being dropped when the OBI equivalents are used instead, its semantics gets lost. The solution would be to assert on its children in OBI that they are part of an OBI:NMR instrument which is done in OBI only for OBI:NMR sample tube, OBI:NMR probe and OBI:magic angle spinning rotor. I wil lmake these asserions for now in nmrCV, but since this is considered axiom injection, we will need to get this into OBI in a separate issue. So this is an interim fix, to get this issue resolved. When these axioms are requested by us in OBI we should also use this opporunity to revise the definitions of these classes, as they are not in line with the OBO Foundry best practices.

@NRayya
Copy link
Contributor Author

NRayya commented Aug 21, 2024

While working on the implementation in https://github.com/nmrML/nmrCV/tree/12-rework-NMR-instrument-manufacturer I noticed that we have a class 'NMR instrument vendor' which is defined as an 'instrument attribute' (basically a piece of information) that seems redundant to me, since we already have 'NMR device manufacturer'. Can we deprecate this as part of this issue?

We have now a separate issue for vendors: #31

@NRayya
Copy link
Contributor Author

NRayya commented Aug 21, 2024

Working on subsuming existing NMR devices under the respective OBI:device branch entails having to deprecate nmrCV classes that were already defined in OBI (coming originally from the nmrCV crew i.e. Daniel Schober). Now, in the current nmrCV some of these NMR devices are grouped under NMRCV:cardinal part of NMR instrument, but as this class is being dropped when the OBI equivalents are used instead, its semantics gets lost. The solution would be to assert on its children in OBI that they are part of an OBI:NMR instrument which is done in OBI only for OBI:NMR sample tube, OBI:NMR probe and OBI:magic angle spinning rotor. I wil lmake these asserions for now in nmrCV, but since this is considered axiom injection, we will need to get this into OBI in a separate issue. So this is an interim fix, to get this issue resolved. When these axioms are requested by us in OBI we should also use this opporunity to revise the definitions of these classes, as they are not in line with the OBO Foundry best practices.

It was decided to do the fixes as needed in nmrCV and contact OBI team so that they import our terms and not the other way around as nmrCV is the NMR specific ontology. This will also solve the axiom injection

@StroemPhi
Copy link
Contributor

To properly document the overlap with OBI, we need to provide a mapping between NMR devices defined in nmrCV and their equivalents in OBI as part of fixing this issue. We will provide this mapping in SSSOM format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Decision regarding this issue was already taken during task force meeting discussion This issue still needs discussion before resolving
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants