IPS-LMU / emuR

The main R package for the EMU Speech Database Management System (EMU-SDMS)
http://ips-lmu.github.io/EMU.html
23 stars 15 forks source link

Docs of rename_attributeDefinition is misleading or is there an error in 'add_attributeDefinition'? #182

Closed FredrikKarlssonSpeech closed 6 years ago

FredrikKarlssonSpeech commented 6 years ago

I am fairly convinced that the docs for 'rename_attributeDefinition()' is not saying the correct thing, but I need guidance on what the correct behavior should be.

The portion of the docs I just saw is saying this:

As the only one of these operations, rename_attributeDefinition can also be used to manipulate (i.e. rename) a level definition. It is therefore not necessary to specify the name of the level that the attribute definition belongs to.

Now, this seems to not be true, as you can certainly insert an attribute on a level that is a duplicate of an attribute of a another level. Like this (using the 'ae' database):

> list_levelDefinitions(ae)
          name    type nrOfAttrDefs        attrDefNames
1    Utterance    ITEM            1          Utterance;
2 Intonational    ITEM            1       Intonational;
3 Intermediate    ITEM            1       Intermediate;
4         Word    ITEM            3 Word; Accent; Text;
5     Syllable    ITEM            1           Syllable;
6      Phoneme    ITEM            1            Phoneme;
7     Phonetic SEGMENT            1           Phonetic;
8         Tone   EVENT            1               Tone;
9         Foot    ITEM            1               Foot;
> add_attributeDefinition(ae,levelName = "Phoneme",name = "Syllable")
  INFO: Rewriting 7 _annot.json files to file system...
  |============================================================================| 100%
> list_levelDefinitions(ae)
          name    type nrOfAttrDefs        attrDefNames
1    Utterance    ITEM            1          Utterance;
2 Intonational    ITEM            1       Intonational;
3 Intermediate    ITEM            1       Intermediate;
4         Word    ITEM            3 Word; Accent; Text;
5     Syllable    ITEM            1           Syllable;
6      Phoneme    ITEM            2  Phoneme; Syllable;
7     Phonetic SEGMENT            1           Phonetic;
8         Tone   EVENT            1               Tone;
9         Foot    ITEM            1               Foot;

Now, calling `

rename_attributeDefinition(ae,"Syllable", "NewSyllable") `

is ambiguous.

So, what should be the correct behavior be here? Should a 'levelName' formal argument should be added to rename_attributeDefinition, or is it the case that attributes of levels really should be unique, and ny 'add_attributeDefinition' call above should really have been stopped.

I think the second option is the intended behavior, as otherwise queries should start being ambiguous. In that case, there should be checks in 'add_attributeDefinition'

raphywink commented 6 years ago

The problem here is that the add_attributeDefinition should not allow for an attribute to be added to any levelDefinition if that attribute name is already taken. Currently attribute definition names have to be unique across the entire emuDB. Thanks for pointing this out... should be a fairly easy fix.