Repository: mobiusklein/psims

Issue:
Compatibility with other tools
This is a great tool! I love it. One suggestion for other users, is the output is not fully compatible with tools taking in mzML. To help with this, simply doing a round trip mzML -> mzML conversion in proteowizard's msconvert totally fixes things. 

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2783b4f..858892b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,18 @@
 All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog][Keep a Changelog] and this project adheres to [Semantic Versioning][Semantic Versioning].
+
+## [v1.2.0] - 2022-07-29
+
+### Changed
+1. When giving a `Spectrum` an ID value which is just an integer, the writer will attempt to convert it into
+   the appropriate nativeID format. To control the nativeID format, set `MzMLWriter.native_id_format` or include
+   an appropriate `cvParam` term in the `MzMLWriter.file_description` parameters.
+
+### Fixed
+1. When a CV fails to import another CV, that failure will be noted and it won't attempt to download again.
+
+
 ## [v1.1.0] - 2022-07-29
 
 | Name | Version | Checksum |
@@ -37,6 +49,25 @@ The format is based on [Keep a Changelog][Keep a Changelog] and this project adh
 
 
 
+## [v.1.1.0] - 2022-07-07
+
+### Added
+- Added `MzMLWriter.native_id_format` attribute that governs how integers are converted into strings for the
+  spectrum id attribute. This will default to `MS:1000774` `multiple peak list nativeID format`. If `fileContents`
+  includes a nativeID format term and it is not been explicitly specified, that format will be used. This has no
+  effect when specifying a a spectrum id with a string.
+
+### Changed
+
+### Deprecated
+
+### Removed
+
+### Fixed
+
+### Security
+
+
 ## [v1.0.0] - 2022-06-15
 
 | Name | Version | Checksum |
diff --git a/makefile b/makefile
index ee734ae..a2615ba 100644
--- a/makefile
+++ b/makefile
@@ -1,5 +1,5 @@
 test:
-	py.test -v -s psims --cov=psims --cov-report=html --cov-report term
+	py.test --log-cli-level DEBUG -v -s psims --cov=psims --cov-report=html --cov-report term
 
 retest:
 	py.test -v psims --lf
diff --git a/psims/controlled_vocabulary/controlled_vocabulary.py b/psims/controlled_vocabulary/controlled_vocabulary.py
index df6fd18..984ac95 100644
--- a/psims/controlled_vocabulary/controlled_vocabulary.py
+++ b/psims/controlled_vocabulary/controlled_vocabulary.py
@@ -334,7 +334,7 @@ def _query_imported(self, query):
                     logger.debug(f"Importing {url} for {self.name}")
                     cv = self.imports[url] = self.import_resolver(url)
                 except ValueError:
-                    cv = None
+                    cv = self.imports[url] = None
             if cv is None:
                 continue
             try:
diff --git a/psims/document.py b/psims/document.py
index 4f1adb1..f6d9065 100644
--- a/psims/document.py
+++ b/psims/document.py
@@ -565,6 +565,9 @@ def term(self, *args, **kwargs):
     def get_vocabulary(self, *args, **kwargs):
         return self.context.get_vocabulary(*args, **kwargs)
 
+    def add_context_key(self, key, value):
+        self.context[key] = value
+
 
 class XMLBindingDispatcherBase(ComponentDispatcherBase):
 
diff --git a/psims/mzml/components.py b/psims/mzml/components.py
index 2fe6fef..5c61cc6 100644
--- a/psims/mzml/components.py
+++ b/psims/mzml/components.py
@@ -114,7 +114,23 @@ def write_content(self, xml_file):
             member.write(xml_file)
 
 
-class FileContent(ComponentBase):
+class HasNativeIDFormat(object):
+
+    @property
+    def native_id_format(self):
+        formats = []
+        for param in self.params:
+            param = self.context.param(param)
+            try:
+                term = self.context.term(param.accession)
+            except (AttributeError, KeyError):
+                continue
+            if term.is_of_type("MS:1000767"):
+                formats.append(term)
+        return formats
+
+
+class FileContent(ComponentBase, HasNativeIDFormat):
     requires_id = False
 
     def __init__(self, params=None, context=NullMap, **kwargs):
@@ -132,8 +148,13 @@ def __init__(self, members, context):
         super(SourceFileList, self).__init__(
             'sourceFileList', members, context)
 
+    @property
+    def native_id_formats(self):
+        formats = list({fid.id: fid for f in self for fid in f.native_id_format}.values())
+        return formats
+
 
-class SourceFile(ComponentBase):
+class SourceFile(ComponentBase, HasNativeIDFormat):
     requires_id = True
 
     def __init__(self, location=None, name=None, id=None,
@@ -204,6 +225,13 @@ def __init__(self, content, source_files, contacts=None, context=NullMap):
         self.context = context
         self.element = _element('fileDescription')
 
+    @property
+    def native_id_formats(self):
+        content_id_format = self.content.native_id_format
+        if content_id_format:
+            return [content_id_format] + self.source_files.native_id_formats
+        return self.source_files.native_id_formats
+
     def write_content(self, xml_file):
         self.content.write(xml_file)
         self.source_files.write(xml_file)
@@ -604,7 +632,8 @@ def __init__(self, index, binary_data_list=None, scan_list=None, precursor_list=
             index=index,
             sourceFileRef=self._source_file_reference,
             defaultArrayLength=self.default_array_length,
-            dataProcessingRef=self._data_processing_reference)
+            dataProcessingRef=self._data_processing_reference,
+            id_formatter=context.get('native_id_formatter'))
         self.context = context
         self.context['Spectrum'][id] = self.element.id
         self.params = self.prepare_params(params, **kwargs)
diff --git a/psims/mzml/native_id.py b/psims/mzml/native_id.py
new file mode 100644
index 0000000..3d03a22
--- /dev/null
+++ b/psims/mzml/native_id.py
@@ -0,0 +1,206 @@
+import re
+
+from typing import OrderedDict, Mapping
+
+
+
+type_pat = re.compile("([A-Za-z]+)=xsd:(%s+)" % '|'.join(
+    {'IDREF', "long", 'nonNegativeInteger', 'positiveInteger', 'string'}))
+
+xsd_to_regex = {
+    "IDREF": r"(\S+)",
+    "long": r"(-?\d+)",
+    "nonNegativeInteger": r"(\d+)",
+    "positiveInteger": r"(\d+)",
+    "string": r"(\S+)",
+}
+
+xsd_to_type = {
+    "IDREF": str,
+    "long": int,
+    "nonNegativeInteger": int,
+    "positiveInteger": int,
+    "string": str,
+}
+
+type_to_default = {
+    "long": 0,
+    "nonNegativeInteger": 0,
+    "positiveInteger": 1,
+}
+
+
+class IDParserBase(object):
+    """A base class for ID parsing and formatting.
+    """
+
+    def __call__(self, text):
+        """Parse a string looking for fields defined by this ID format.
+
+        Parameters
+        ----------
+        text : str
+            The string to parse
+
+        Returns
+        -------
+        dict:
+            The parsed fields of the ID string.
+
+        See Also
+        --------
+        :meth:`parse`
+        """
+        return self.parse(text)
+
+
+class NativeIDParser(IDParserBase):
+    """A parser for a single nativeID format.
+
+    These may be automatically derived from the CV-param defining them by parsing the
+    XSD string included, but no guarantee is available.
+    """
+
+    def __init__(self, parser, tokens, name):
+        self.parser = parser
+        self.tokens = OrderedDict(tokens)
+        self.name = name
+
+    @classmethod
+    def from_term(cls, term):
+        """Construct a :class:`NativeIDParser` from :class:`IDFormat` term.
+
+        Parameters
+        ----------
+        term : IDFormat
+            The nativeID format specification to build a parser for
+
+        Returns
+        -------
+        :class:`NativeIDParser`:
+            The constructed parser, or :const:`None` if no regular expression could be
+            constructed.
+        """
+        if "Native format defined by" in term.definition:
+            tokens = []
+            desc = term.definition.split(
+                "Native format defined by", 1)[1].rstrip()
+            for mat in type_pat.finditer(desc):
+                tokens.append(mat.groups())
+            parser = re.compile(
+                ''.join([r"(%s)=%s\s?" % (k, xsd_to_regex[v]) for k, v in tokens]))
+            return cls(parser, tokens, term.name)
+        return None
+
+    def parse(self, string):
+        """Parse a string according to this parser's pattern,
+        returning the type-cast fields as a :class:`dict`.
+
+        Parameters
+        ----------
+        string : str
+            The string to parse
+
+        Returns
+        -------
+        dict
+            The fields of the scan ID
+
+        Raises
+        ------
+        ValueError:
+            If the string does not conform to the expected pattern
+        """
+        match = self.parser.match(string)
+        if match is None:
+            return OrderedDict()
+        groups = match.groups()
+        n = len(groups)
+        i = 0
+        fields = OrderedDict()
+        while i < n:
+            k = groups[i]
+            v = groups[i + 1]
+            i += 2
+            try:
+                v = int(v)
+            except ValueError:
+                pass
+            fields[k] = v
+        return fields
+
+    def format(self, fields):
+        """Format a set of fields as a nativeID string.
+
+        Parameters
+        ----------
+        fields : dict
+            The fields to populate the nativeID from.
+
+        Returns
+        -------
+        str
+        """
+        parts = []
+        for key in self.tokens:
+            parts.append("%s=%s" % (key, fields[key]))
+        return ' '.join(parts)
+
+    def format_integer(self, value):
+        tokens = list(self.tokens.items())
+        fields = {}
+        n = len(tokens) - 1
+        for i, (name, tp) in enumerate(tokens):
+            if i == n:
+                fields[name] = value
+            else:
+                if tp in type_to_default:
+                    fields[name] = type_to_default[tp]
+                else:
+                    raise ValueError(f"Cannot infer nativeID format default for {name} of type {tp}")
+        return self.format(fields)
+
+
+
+class MultipleIDFormats(Mapping, IDParserBase):
+    '''Represent an ambiguous group of multiple nativeID formats.
+
+    Implements the :class:`~collections.abc.Mapping` interface.
+
+    Attributes
+    ----------
+    id_formats : OrderedDict
+        A mapping of format name to :class:`IDFormat` instances
+    '''
+
+    def __init__(self, id_formats):
+        self.id_formats = id_formats
+
+    def parse(self, text):
+        fields = OrderedDict()
+        for name, parser in self.id_formats.items():
+            fields = parser.parse(text)
+            if not fields:
+                continue
+            else:
+                fields['id_format'] = name
+                break
+        return fields
+
+    def format(self, fields):
+        format_name = fields.get('id_format')
+        id_format = self.id_formats[format_name]
+        return id_format.format(fields)
+
+    def __iter__(self):
+        return iter(self.id_formats)
+
+    def __getitem__(self, key):
+        return self.id_formats[key]
+
+    def __len__(self):
+        return len(self.id_formats)
+
+    def __repr__(self):
+        template = "{self.__class__.__name__}({self.id_formats})"
+        return template.format(self=self)
diff --git a/psims/mzml/writer.py b/psims/mzml/writer.py
index 882af08..9c36649 100644
--- a/psims/mzml/writer.py
+++ b/psims/mzml/writer.py
@@ -18,17 +18,18 @@
 import numbers
 import warnings
 
-from typing import List, Optional, Dict, Tuple, Mapping, Iterable, BinaryIO, TypedDict, Union, DefaultDict
+from typing import List, Optional, Dict, Tuple, Mapping, Iterable, TypedDict, Union, DefaultDict
 
 from collections import defaultdict
 
 import numpy as np
 
 from psims.xml import CVParam, XMLWriterMixin, XMLDocumentWriter
+from psims.controlled_vocabulary import Entity
 from psims.utils import TableStateMachine
 
 from .components import (
-    ComponentDispatcher, Software, Spectrum, element,
+    ComponentDispatcher, FileDescription, Software, Spectrum, element,
     default_cv_list, MzML, InstrumentConfiguration, IndexedMzML)
 
 from .binary_encoding import (
@@ -37,6 +38,7 @@
 
 from .utils import ensure_iterable
 from .index import IndexingStream
+from .native_id import NativeIDParser
 
 from .element_builder import ElementBuilder, ParamManagingProperty
 
@@ -101,6 +103,10 @@ def __init__(self, section, writer, parent_context, section_args=None, **kwargs)
         self.writer = writer
         self.section_args = section_args
 
+    @property
+    def native_id_format(self):
+        return self.writer.native_id_format
+
     def __enter__(self):
         return self.begin()
 
@@ -230,9 +236,12 @@ class PlainMzMLWriter(ComponentDispatcher, XMLDocumentWriter):
     DEFAULT_INTENSITY_UNIT = DEFAULT_INTENSITY_UNIT
 
     def __init__(self, outfile, close=None, vocabularies=None, missing_reference_is_error=False,
-                 vocabulary_resolver=None, id=None, accession=None, **kwargs):
+                 vocabulary_resolver=None, id=None, accession=None, native_id_format: str=None,
+                 **kwargs):
         if vocabularies is None:
             vocabularies = []
+        if native_id_format is None:
+            native_id_format = "multiple peak list nativeID format"
         vocabularies = list(default_cv_list) + list(vocabularies)
         ComponentDispatcher.__init__(
             self,
@@ -259,6 +268,56 @@ def __init__(self, outfile, close=None, vocabularies=None, missing_reference_is_
             ('spectrum_list', ['chromatogram_list']),
             ('chromatogram_list', [])
         ])
+        self._native_id_format = self._find_native_id_parser(native_id_format)
+        self.native_id_format_configured = False
+        self.add_context_key('native_id_formatter', self._native_id_maker)
+
+    @property
+    def native_id_format(self):
+        '''The nativeID format of the spectra to assume for this data file.
+
+        This is used to determine how to convert an integer into a spectrum's :attr:`~.Spectrum.id`.
+        Defaults to ``MS:1000774``: "multiple peak list nativeID format" which has a pattern of
+        ``index=<number>``.
+
+        This attribute has no effect on spectrum id values specified as strings already formatted.
+
+        .. note::
+            If not explicitly specified, but a term naming an ID format is passed as a parameter in
+            file contents, that will be used. The ID format from source files will **not** be used.
+
+        Returns
+        -------
+        :class:`~.NativeIDParser`
+        '''
+        return self._native_id_format
+
+    @native_id_format.setter
+    def native_id_format(self, native_id_format: str):
+        '''Set the nativeID format to use for this file.
+
+        You can specify the name the ID format using either the format's name
+        or its CV ID, e.g. "MS:1000774", or a :class:`~psims.controlled_vocabulary.entity.Entity`
+        describing the same term.
+
+        Explicitly setting this attribute will *prevent* automatically using
+        the nativeID format specified in ``<fileContent>``.
+
+        Parameters
+        ----------
+        native_id_format : :class:`~.Union`[str, :class:`~psims.controlled_vocabulary.entity.Entity`]
+            The nativeID format to use for this file.
+        '''
+        self._native_id_format = self._find_native_id_parser(native_id_format)
+        self.native_id_format_configured = True
+
+    def _find_native_id_parser(self, name: Union[str, Entity]) -> NativeIDParser:
+        if not isinstance(name, Entity):
+            term = self.term(name)
+        return NativeIDParser.from_term(term)
+
+    def _native_id_maker(self, _tag_name, number):
+        return self._native_id_format.format_integer(number)
 
     def toplevel_tag(self):
         return MzML(id=self.id, accession=self.accession)
@@ -294,6 +353,9 @@ def software_list(self, software_list: Iterable[Union[Software, Mapping]]):
     def file_description(self, file_contents=None, source_files=None, contacts=None):
         r"""Writes the ``<fileDescription>`` section of the document.
 
+        If ``file_contents`` contains a nativeID term, and :attr:`native_id_format` has
+        not been set explicitly, that ID format will be used for this document.
+
         .. note::
             Information pertaining to the entire mzML file (i.e. not specific
             to any part of the data set) is stored here.
@@ -308,9 +370,14 @@ def file_description(self, file_contents=None, source_files=None, contacts=None)
             to be placed in the ``<sourceFileList>`` element
         """
         self.state_machine.transition("file_description")
-        fd = self.FileDescription(
+        fd: FileDescription = self.FileDescription(
             file_contents, [self.SourceFile.ensure(sf) for sf in ensure_iterable(source_files)],
             contacts=[self.Contact.ensure(c) for c in ensure_iterable(contacts)])
+        native_id_format = fd.content.native_id_format
+        if native_id_format and not self.native_id_format_configured:
+            self.native_id_format = NativeIDParser.from_term(native_id_format)
+        elif self.native_id_format_configured:
+            fd.content.add_param(self.native_id_format.name)
         fd.write(self.writer)
 
     def instrument_configuration_list(self, instrument_configurations):
diff --git a/psims/version.py b/psims/version.py
index 2a7accc..ee965d3 100644
--- a/psims/version.py
+++ b/psims/version.py
@@ -1,1 +1,1 @@
-version = '1.1.1'
\ No newline at end of file
+version = '1.2.0'
diff --git a/psims/xml.py b/psims/xml.py
index b7016e0..e372010 100644
--- a/psims/xml.py
+++ b/psims/xml.py
@@ -162,6 +162,7 @@ class TagBase(object):
     def __init__(self, tag_name=None, text="", **attrs):
         self.tag_name = tag_name or self.tag_name
         _id = attrs.pop('id', None)
+        _id_formatter = attrs.pop('id_formatter', id_maker)
         self.attrs = {}
         self.attrs.update(self.type_attrs)
         self.text = text
@@ -170,6 +171,7 @@ def __init__(self, tag_name=None, text="", **attrs):
         # and any set ids will be passed through the attrs dictionary, but the `with_id`
         # flag won't be propagated. `_force_id` preserves this.
         self._force_id = True
+        self._id_formatter = _id_formatter
         if _id is None:
             self._id_number = None
             self._id_string = None
@@ -210,7 +212,7 @@ def get(self, name, default):
     @property
     def id(self):
         if self._id_string is None and self._id_number is not None:
-            self._id_string = id_maker(self.tag_name, self._id_number)
+            self._id_string = self._id_formatter(self.tag_name, self._id_number)
         return self._id_string
 
     def element(self, xml_file=None, with_id=False):


Test patch:
diff --git a/psims/test/test_mzml_writer.py b/psims/test/test_mzml_writer.py
index 8df173a..23a4734 100644
--- a/psims/test/test_mzml_writer.py
+++ b/psims/test/test_mzml_writer.py
@@ -104,8 +104,11 @@ def test_write(output_path, compressor):
     with MzMLWriter(compressor(output_path, 'wb'), close=None) as f:
         f.register("Software", 'psims')
         f.controlled_vocabularies()
+        f.native_id_format = "Agilent MassHunter nativeID format"
         f.file_description(["spam", "MS1 spectrum", "MSn spectrum"], [
-            dict(id="SPAM1", name="Spam.raw", location="file:///", params=[dict(name="Thermo RAW format")])])
+            dict(id="SPAM1", name="Spam.raw", location="file:///", params=[
+                dict(name="Thermo RAW format"), dict(name="Agilent MassHunter nativeID format")])
+                ])
         f.reference_param_group_list([
             {'id': 'common_params', 'params': [{"proven": "inductively"}]}
         ])
@@ -132,7 +135,7 @@ def test_write(output_path, compressor):
         ])
         with f.run(id='test'):
             with f.spectrum_list(count=2):
-                f.write_spectrum(mz_array, intensity_array, charge_array, id='scanId=1', params=[
+                f.write_spectrum(mz_array, intensity_array, charge_array, id=1, params=[
                     {"name": "ms level", "value": 1}, {"ref": 'common_params'}],
                     polarity='negative scan', encoding=encodings,
                     compression='zlib')
@@ -142,7 +145,7 @@ def test_write(output_path, compressor):
                         "mz": 1230, "intensity": None, "charge": None, "params": [
                             "no peak detected"
                         ],
-                        "scan_id": "scanId=1", "activation": ["collision-induced dissociation",
+                        "scan_id": 1, "activation": ["collision-induced dissociation",
                                                               {"collision energy": 56.}]
                 }, instrument_configuration_id=2, encoding=encodings, compression='zlib')
                 pb = f.precursor_builder(mz=12030, scan_id='scanId=2')