Airr2akc refactor

[LonnekeScheffer]

Major refactoring of airr2akc script: resolves issues raised in airr-knowledge/issues#28

• add parameter to control version prefix (i.e. flag to use Info version or not), by default no prefix on class/slot names.
• remove class name as prefix to slots, let's get list of conflicts, we might just skip output (or change AIRR standards)
• check v1.5 output, remove local yaml
• add Makefile targets for default and v2.0 output, output versions into different files
• With no prefix, there might be conflicts with the current data model. These need to be resolved.
• Need to verify any changes don't break Brian's data transform script.
• Need to verify any changes don't break James's data transform script.
• Have Scott or Brian perform any AIRR standards changes

[github-actions]

PR Preview Action v1.4.8
🚀 Deployed preview to https://airr-knowledge.github.io/ak-schema/pr-preview/pr-18/
on branch gh-pages at 2024-12-14 20:13 UTC

[LonnekeScheffer]

attempt to reopen PR to trigger automatic testing

[LonnekeScheffer]

Note: in the current solution, all slots are prefixed with the class name, because this is the only way it's possible to specify different values for 'required' (being required for some classes and not for others)
Enums and ontologies are still shared across classes (may either need to change that here or fix it in the airr schema input; 'Unit' has different meaning in different classes)

[schristley]

Hi @LonnekeScheffer , I've added submodules for v1.5 and v2.0 of airr-standards so now those specs can be referenced directly instead of committing a local copy. Once you pull these changes, you will need to do an extra command to initialize the submodules (the README has it: git submodule update --init --recursive).

Also I note that you were working from the v2.0 schema, and for now we want to use v1.5, so I changed the Makefile to reflect that. I did not regenerate the output. I will let you do that so you can verify everything is working properly.

[schristley]

Note: in the current solution, all slots are prefixed with the class name, because this is the only way it's possible to specify different values for 'required' (being required for some classes and not for others) Enums and ontologies are still shared across classes (may either need to change that here or fix it in the airr schema input; 'Unit' has different meaning in different classes)

Regarding required, what is required in the AKC data model will be different from what is required in the AIRR standards, so instead of multiple slots, I would just drop the required when there is a conflict.

[schristley]

Hi @LonnekeScheffer , I've added the last bit to be done on the top post, then we can merge this and do a release.

[schristley]

@bcorrie I got a little closer by 1) fixed bug with Dockerfile to install jq, 2) creating studies directory, 3) git clone https://github.com/sfu-ireceptor/config.git then changing branch to AKC, and 4) generating ak_schema.py

bash akc-convert-study.sh ipa1.ireceptor.org PRJNA248411 studies
Info: Processing AIRR repertoire metadata file: studies/ipa1.ireceptor.org-PRJNA248411-ADC.json
Traceback (most recent call last):
  File "/work/src/scripts/akc-convert/dataloader.py", line 130, in <module>
    if not parser.checkValidity():
  File "/usr/local/lib/python3.9/site-packages/jsonasobj2/_jsonobj.py", line 160, in __getattr__
    return super().__getattribute__(item)
AttributeError: 'AIRRRepertoire' object has no attribute 'checkValidity'
Error: Conversion of ADC query response failed

[schristley]

I'm able to run on main branch successfully, and verified all the latest code is on this branch, so it must be some difference in the schema causing the problem, but sure why. My guess is a name conflict...

[schristley]

ok, that was part of it, conversion script had a "Repertoire" class which was conflicting with Repertoire brought over from AIRR schema. So now the error has something to do with an enum

bash akc-convert-study.sh ipa1.ireceptor.org PRJNA248411 studies
Info: Processing AIRR repertoire metadata file: studies/ipa1.ireceptor.org-PRJNA248411-ADC.json
Info: Processing AKC classes: ['Investigation' 'StudyArm' 'Reference' 'Participant' 'ImmuneExposure'
 'Specimen' 'LifeEvent' 'CellIsolationProcessing' 'NucleicAcidProcessing'
 'LibraryPreparationProcessing' 'ReceptorRepertoireSequencingAssay']
Traceback (most recent call last):
  File "/work/src/scripts/akc-convert/dataloader.py", line 135, in <module>
    parse_ok = parser.process(options.filename, out_file)
  File "/work/src/scripts/akc-convert/airr_repertoire.py", line 75, in process
    investigation_dict = self.generateAKCInvestigation(repertoire_dict, investigation_dict, akc_class_list) 
  File "/work/src/scripts/akc-convert/repertoire.py", line 400, in generateAKCInvestigation
    akc_object = globals()[akc_class]('')
  File "<string>", line 13, in __init__
  File "/work/src/scripts/akc-convert/ak_schema.py", line 2893, in __post_init__
    self.template_class = TemplateClassEnum(self.template_class)
  File "/usr/local/lib/python3.9/site-packages/linkml_runtime/utils/enumerations.py", line 52, in __init__
    raise ValueError(f"Unknown {self.__class__.__name__} enumeration code: {key}")
ValueError: Unknown TemplateClassEnum enumeration code: 
Error: Conversion of ADC query response failed

[bcorrie]

@schristley @LonnekeScheffer you are doing this on the airrakc_refactor branch, after all of the specimen processing code has been merged in, is that correct? As far as I am aware, the branches that I was working on have been merged in...

I can take a look at it. I can see why there might be conflicts in classes etc if all of the AIRR objects are now generating python classes (e.g. The Repertoire case above). In addition, if the Consolidated Spreadsheet is not up to date that may also cause some problems in the case where a slot is incorrectly being identified as referring to an external class.

[schristley]

@schristley @LonnekeScheffer you are doing this on the airrakc_refactor branch, after all of the specimen processing code has been merged in, is that correct? As far as I am aware, the branches that I was working on have been merged in...

Yes

I can take a look at it. I can see why there might be conflicts in classes etc if all of the AIRR objects are now generating python classes (e.g. The Repertoire case above). In addition, if the Consolidated Spreadsheet is not up to date that may also cause some problems in the case where a slot is incorrectly being identified as referring to an external class.

I turned on debugging and can see it gets the error when trying to create a NucleicAcidProcessing. The old NucleicAcidProcessing class is commented out so it is now the AIRR one, which has a few more fields in it, but template_class and it's enum seem to be same, so not sure why it is complaining about it. Also this study has the template_class set to RNA for all of the repertoires, so it's not like there is a null...

Info: Create instance NucleicAcidProcessing_43213_43213_CSF_392, field = template_class, value = RNA, class = NucleicAcidProcessing, type = string, airr class = SampleProcessing

[schristley]

Another potential issue is that the converted AIRR classes aren't placed within the inheritance hierarchy, i.e. they don't have is_a so the class might be missing some fields. I don't think that is the problem for this specific error. My plan to resolve that is to use mixins for composing some classes, like the specimen processing classes.

[bcorrie]

I am not sure how well it will handle the Enums. Let me check. If you look in the consolidated spreadsheet, there is a column that talks about "translation" - this is where an AIRR field actually requires some processing to convert the AIRR data to LinkML. Enum's are not likely to be 1-1, in the sense that in the AIRR world a string (e.g. "contains_schema_rearrangement") will need to be converted to an instance of a LinkML Enum (e.g. KeywordsStudyEnum) with value "contains_schema_rearrangement".

Currently, the converter will just try to copy the string to slot or if the slot is denoted as an Enum in the consolidated spreadsheet it will probably create the class (which is what it seems to be doing) but then when it tries to assign the string it is failing ("ValueError: Unknown TemplateClassEnum enumeration code: "). So it looks to me like the mechanism to set a value for a LinkML enum code is not correct.

Do you want me to investigate and make commits on this branch?

[bcorrie]

Same is true of AIRR Ontologies. Right now, the converter is actually creating a YAML object that is the equivalent of the AIRR Ontology object (with and id and label) rather than creating the correct Enum object with the id being assigned to the Enum value.

I think we should consider whether or not we just keep the ontology ID. By using a simple Enum for an ontology object we are discarding the label info. We certainly want to have the ontology ID, but it isn't clear to me at all as to whether we should drop the ontology label? I would suggest it isn't needed, but it sure is nice to have whenever you want to do something user facing (where the ID is meaningless). Sure, you can have every consumer of the AKC data look up all of the IDs, but that is a large burden for something that is pretty trivial to store up front...

Both the ADC and IEDB keep the id and the label for many key entities. For example see:

curl -s https://query-api.iedb.org/tcr_search?chain2_cdr3_seq=like.CASSLQSSYNSPLHF | jq

In the ADC there is a label element for all Ontology based fields.

[schristley]

Do you want me to investigate and make commits on this branch?

Yes please, I'm still investigating as well. From the error message, it suggests to me that it is trying to set a blank value for the template_class field?

From what I can tell, there is no enum for template_class on the main branch. Here is the slot:

  template_class:
    description: The class of nucleic acid that was used as primary starting material for the following procedures
    # TODO: enum

but now there is an enum attached to template_class

  template_class:
    name: template_class
    description: The class of nucleic acid that was used as primary starting material for the following procedures
    range: TemplateClassEnum
    annotations:
      nullable: false

[schristley]

I think we should consider whether or not we just keep the ontology ID. By using a simple Enum for an ontology object we are discarding the label info. We certainly want to have the ontology ID, but it isn't clear to me at all as to whether we should drop the ontology label?

Just the ID needs to be brought over. The labels will be available, just not through the schema. Instead we will have database tables with ID and labels that come from our ak-ontology repository (not integrated completely yet).

[schristley]

@bcorrie ok, I can manually reproduce in python. The issue seems to be that when creating an initial blank NucleicAcidProcessing, it wants a value for template class. Here is were I simulate your conversion code by creating an empty object

>>> from linkml_runtime.dumpers import yaml_dumper, json_dumper, tsv_dumper
>>> from ak_schema import *
>>> x = NucleicAcidProcessing('')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<string>", line 13, in __init__
  File "/work/src/scripts/akc-convert/ak_schema.py", line 2893, in __post_init__
    self.template_class = TemplateClassEnum(self.template_class)
  File "/usr/local/lib/python3.9/site-packages/linkml_runtime/utils/enumerations.py", line 52, in __init__
    raise ValueError(f"Unknown {self.__class__.__name__} enumeration code: {key}")
ValueError: Unknown TemplateClassEnum enumeration code: 

however if I create it with a template_class, then it is ok

>>> x = NucleicAcidProcessing(template_class='RNA')
>>> x
NucleicAcidProcessing({'template_class': TemplateClassEnum(text='RNA')})

[schristley]

I'm not sure why it is complaining about template_class specifically, it is not required. Plus NucleicAcidProcessing has other fields which are enums, like library_generation_method, and those seem fine as blank

[bcorrie]

Just the ID needs to be brought over. The labels will be available, just not through the schema. Instead we will have database tables with ID and labels that come from our ak-ontology repository (not integrated completely yet).

That is fine for our user interface and if using the DB, but if anyone is using the AKC using the query API or outputs data into a file using that API, they won't have the ontology labels. So by having the ontology labels in the AKC repository we are making our lives easier (in particular for our user interface), but we aren't making the lives of the consumer of the data in the AKC easier.

Not necessarily against what you are suggesting, but playing devils advocate from the point of view of the end user of the AKC API.

[bcorrie]

I'm not sure why it is complaining about template_class specifically, it is not required. Plus NucleicAcidProcessing has other fields which are enums, like library_generation_method, and those seem fine as blank

I have seen similar weird behaviour where some LinkML classes require things that others don't. If you look at the files generated by adc_convert (e.g. examples/adc/ipa1.ireceptor.org-PRJNA248411-AKC.json) you will see there is an empty investigation with an empty akc_id.

  "AIRRKnowledgeCommons": {
    "investigations": {
      "": {
        "akc_id": ""
      },
      "0ccdc7a1-04be-4c63-a24a-a1c26e1884bc": {

I have no idea why this is created and I can't seem to avoid its creation when I create an AIRRKnowledgeCommons object.

[schristley]

Just the ID needs to be brought over. The labels will be available, just not through the schema. Instead we will have database tables with ID and labels that come from our ak-ontology repository (not integrated completely yet).

That is fine for our user interface and if using the DB, but if anyone is using the AKC using the query API or outputs data into a file using that API, they won't have the ontology labels. So by having the ontology labels in the AKC repository we are making our lives easier (in particular for our user interface), but we aren't making the lives of the consumer of the data in the AKC easier.

Not necessarily against what you are suggesting, but playing devils advocate from the point of view of the end user of the AKC API.

Well, I guess I didn't explain myself well, the labels will be in AKC repository. They will be accessible from the query API.

[bcorrie]

I'm not sure why it is complaining about template_class specifically, it is not required. Plus NucleicAcidProcessing has other fields which are enums, like library_generation_method, and those seem fine as blank

It seems to me that it would be reasonable for an Enum to require a value, since Enums probably shouldn't be created without a valid enum value I would assume??? So that behavior wasn't too surprising for me, but my code would need to make sure that happens.

So maybe the question is why can you create a LibraryGenerationMethodEnum without a value 8-)

[schristley]

@bcorrie ok, I think I have it partially figured out. When creating an instance like this:

x = NucleicAcidProcessing('')

The blank string parameter is actually being assigned to the first slot in the class, which happens to be template_class! So the blank string, which is treated differently from None for no value, is checked against the enum and throws the error. If you create the object like this then it creates it without error

x = NucleicAcidProcessing()

I replaced every instance in repertoire.py, which gets it past that error, but then there is a new error complaining about akc_id being a required field.

[schristley]

That makes a bit more sense now too. Before with (''), that was being assigned to akc_id for many objects, thus why the required field error wasn't being thrown. So instead of assigning the akc_id afterwards, probably need to initialize the objects like this:

akc_object = globals()[akc_class](akc_id=str(uuid.uuid4()))

[schristley]

Some of the classes need to be changed. I didn't realize this before but the AIRR NucleicAcidProcessing slots were split out across multiple classes. This isn't necessary as they all belong under LibraryPreparationProcessing. Likewise ReceptorRepertoireSequencingAssay should be slots from AIRR SequencingRun.

[bcorrie]

Some of the classes need to be changed. I didn't realize this before but the AIRR NucleicAcidProcessing slots were split out across multiple classes. This isn't necessary as they all belong under LibraryPreparationProcessing. Likewise ReceptorRepertoireSequencingAssay should be slots from AIRR SequencingRun.

This is getting a bit off topic, as we are talking data modeling - maybe we need a new issue for this...

My understanding was that we were trying to model these processes better than what we have in the AIRR Standard by using the modelling diagram that we have in the Miro board and trying to create a more precise and flexible model of how samples and data are processed. It is certainly less work (and therefore translation is easier) to just use the AIRR classes, but I thought we were trying to improve the model???

That is what I tried to do when I split these out in #12 . There are different OBI processes (cell processing, sequencing preparation, and library preparation - see the LinkML definitions for these classes) for each of these types of processing. Now we don't have to model these if we don't want, but to me the mapping was pretty natural and having them separate could be beneficial. So that is why the AIRR slots are spread out over multiple classes in the AKC LinkML. There is a 1:1 mapping (for the most part) of the actual slots, there is just not a 1:1 mapping of ADC class to AKC class.

For the ReceptorRepertoireSequencingAssay I would have thought that this class described the assay itself, not how and when the Assay was applied to a set of samples (what I think of as an AIRR SequencingRun) and what it produced (the AIRR SequencingData).

I don't think we have anything modelled in LinkML after the Assay in the Miro board (which in my mind is the current ReceptorRepertoireSequencingAssay for AIRR-seq). In the Miro board, an Assay has "specified output" of Data. So for me the AIRR SequencingRun describes how an Assay is applied to a Specimen that had SpecimenProcessing applied, and the output of that would be SequencingData 8-)

We can certainly use the AIRR classes for this directly if we want, in fact that is what I have as a place holder in the "Consolidate Mapping" sheet. But I would not have thought that those slots would be part of ReceptorRepertoireSequencingAssay.

[schristley]

I'm not sure why it is complaining about template_class specifically, it is not required. Plus NucleicAcidProcessing has other fields which are enums, like library_generation_method, and those seem fine as blank

I have seen similar weird behaviour where some LinkML classes require things that others don't. If you look at the files generated by adc_convert (e.g. examples/adc/ipa1.ireceptor.org-PRJNA248411-AKC.json) you will see there is an empty investigation with an empty akc_id.

  "AIRRKnowledgeCommons": {
    "investigations": {
      "": {
        "akc_id": ""
      },
      "0ccdc7a1-04be-4c63-a24a-a1c26e1884bc": {

I have no idea why this is created and I can't seem to avoid its creation when I create an AIRRKnowledgeCommons object.

Fixing the initialization of the classes has eliminated this.

[schristley]

While changing the initialization works for most classes, it doesn't work for Reference because ForeignObject does not have an akc_id, instead its identifier is source_uri. The result is that there is an initialization error when the convert script tries to create these objects.

Info: Create instance Reference_PRJNA248411, field = sources, value = PMID: 25100740, class = Reference, type = uriorcurie, airr class = Study
Traceback (most recent call last):
  File "/work/src/scripts/akc-convert/dataloader.py", line 135, in <module>
    parse_ok = parser.process(options.filename, out_file)
  File "/work/src/scripts/akc-convert/airr_repertoire.py", line 75, in process
    investigation_dict = self.generateAKCInvestigation(repertoire_dict, investigation_dict, akc_class_list) 
  File "/work/src/scripts/akc-convert/repertoire.py", line 400, in generateAKCInvestigation
    akc_object = globals()[akc_class](akc_id=str(uuid.uuid4()))
  File "<string>", line 13, in __init__
  File "/work/src/scripts/akc-convert/ak_schema.py", line 641, in __post_init__
    self.MissingRequiredField("source_uri")
  File "/usr/local/lib/python3.9/site-packages/linkml_runtime/utils/yamlutils.py", line 281, in MissingRequiredField
    raise ValueError(f"{field_name} must be supplied")
ValueError: source_uri must be supplied
Error: Conversion of ADC query response failed

@bcorrie I'm not sure of the best way to resolve this in the convert script. Essentially we need some flag or info that lets us know how to init and/or we handle Reference specially. The latter may be better because I think this is the only class where it is identified by its external ID, e.g. PMID:12345

As a quick hack to test the rest of the conversion, can change the is_a for Reference from ForeignObject to NamedThing in ak_study_design.yaml

--- a/src/ak_schema/schema/ak_study_design.yaml
+++ b/src/ak_schema/schema/ak_study_design.yaml
@@ -23,7 +23,7 @@ classes:
       - conclusions
 
   Reference:
-    is_a: ForeignObject
+    is_a: NamedThing
     class_uri: IAO:0000310 # document
     # See https://schema.org/ScholarlyArticle
     description: >-

[schristley]

Some of the classes need to be changed. I didn't realize this before but the AIRR NucleicAcidProcessing slots were split out across multiple classes. This isn't necessary as they all belong under LibraryPreparationProcessing. Likewise ReceptorRepertoireSequencingAssay should be slots from AIRR SequencingRun.

This is getting a bit off topic, as we are talking data modeling - maybe we need a new issue for this...

It's worthwhile to have these discussions, though I'd prefer to do them in schema KG or other appropriate meeting versus writing lots of words. We are talking about semantics so it's better to talk it out.

I committed a change to ak_specimens.yaml to the data model that I'm thinking. I also changed the name of ReceptorRepertoireSequencingAssay to AIRRSequencingAssay to be more specific. I also changed the consolidated spreadsheet, though I don't know how to convert that to the mapping file. I've tried manually modifying the mapping file to test but I'm not really sure that I've changed it properly.

[schristley]

@bcorrie if you put the Reference hack in place, then your convert tool runs successfully, though I broke it again by reorganizing and renaming those classes. Once the mapping is resolved it should work. I think I made the appropriate changes to the consolidated spreadsheets but can you please review and try with an updated mapping file.

[schristley]

needed to change the name in convert scripts too, with the Reference hack the conversion runs completely, but still need to verify that the output is correct.

[schristley]

hi @jamesaoverton , your IEDB conversion sample runs ok with the changes on this branch, except for the conversion to TTL. There is this error, which isn't very informative. Maybe something isn't conformant to the grammatical rules for TTL. Can you please take a look?

FYI- you will want to do a make all at top level to insure the generated files are up-to-date

linkml-convert -s ../../project/linkml/ak_schema.yaml -C AIRRKnowledgeCommons -t rdf example.yaml > example.ttl
Traceback (most recent call last):
  File "/usr/local/bin/linkml-convert", line 8, in <module>
    sys.exit(cli())
  File "/usr/local/lib/python3.9/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
  File "/usr/local/lib/python3.9/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
  File "/usr/local/lib/python3.9/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
  File "/usr/local/lib/python3.9/site-packages/linkml/utils/converter.py", line 178, in cli
    print(dumper.dumps(obj, **outargs))
  File "/usr/local/lib/python3.9/site-packages/linkml_runtime/dumpers/rdflib_dumper.py", line 186, in dumps
    return self.as_rdf_graph(element, schemaview, prefix_map=prefix_map).\
  File "/usr/local/lib/python3.9/site-packages/linkml_runtime/dumpers/rdflib_dumper.py", line 68, in as_rdf_graph
    self.inject_triples(element, schemaview, g)
  File "/usr/local/lib/python3.9/site-packages/linkml_runtime/dumpers/rdflib_dumper.py", line 141, in inject_triples
    v_node = self.inject_triples(v, schemaview, graph, slot.range)
  File "/usr/local/lib/python3.9/site-packages/linkml_runtime/dumpers/rdflib_dumper.py", line 141, in inject_triples
    v_node = self.inject_triples(v, schemaview, graph, slot.range)
  File "/usr/local/lib/python3.9/site-packages/linkml_runtime/dumpers/rdflib_dumper.py", line 89, in inject_triples
    element = element.code
AttributeError: 'extended_str' object has no attribute 'code'