UofS-Pulse-Binfo / nd_genotypes

Drupal/Tripal: Tripal interface for Chado Natural Diversity Genotypic data.
https://nd-genotypes.readthedocs.io/en/latest/index.html
GNU General Public License v2.0
4 stars 1 forks source link

SO bundle terms dont match Tripal defaults #34

Closed bradfordcondon closed 5 years ago

bradfordcondon commented 5 years ago

bundles look for SO terms before attaching. For example, before attaching to Sequence Variant, it looks for sequence_variant


  // FEATURE-BASED CONTENT TYPES.
  /////////////////////////////////////
  if (isset($bundle->data_table) AND ($bundle->data_table == 'feature')) {

    dpm($bundle);
    dpm($term);

    // Variant Genotype Summary Field.
    //---------------------------------
    // Only add the Variant Genotype Summary field
    // if the current Tripal Content Type is SO:sequence_variant.
    if ($term->name == 'sequence_variant') {

Unfortunately, Tripal installs the terms as Sequence Variant. This means the fields never attach. You can see this by my dump of the TripalTerm object (from a freshly installed tripal site):

Screen Shot 2019-05-11 at 10 41 24 PM

heres the chado install of hte entity type (and the wrong term name)

  //
  // Create the 'Sequence Variant' entity type.
  //
  $args = [
    'vocabulary' => 'SO',
    'accession' => '0001060',
    'term_name' => 'Sequence Variant',
    'storage_args' => [
      'data_table' => 'feature',
      'type_column' => 'type_id',
    ],
    'category' => 'Genetic',
  ];

Which is correct, Sequence Variant or sequence_variant? Looks to me like sequence_variant. If so, we should correct in core? Finally a slapdash fix can look something like this:

    if ($term->name == 'sequence_variant' || $term->name == 'Sequence Variant') {

or, maybe you should check by accession instead?

laceysanderson commented 5 years ago

Interesting... I should likely check by accession. That said, we should likely get this fixed in Tripal Core as well since I believe the term name should match the cvterm.name (sequence_variant)

bradfordcondon commented 5 years ago

Submitted a PR to core to resolve, if merged this module will require no changes.

bradfordcondon commented 5 years ago

merged in core.