diff --git a/src/formpack/schema/fields.py b/src/formpack/schema/fields.py index 428c0577..1878a334 100644 --- a/src/formpack/schema/fields.py +++ b/src/formpack/schema/fields.py @@ -253,13 +253,13 @@ def from_json_definition( 'select_one_external': partial(TextField, data_type=data_type), 'cascading_select': partial(TextField, data_type=data_type), # qualitative analysis and NLP - 'qual_auto_keyword_count': QualField, - 'qual_integer': QualNumField, - 'qual_note': QualField, - 'qual_select_multiple': QualSelectMultipleField, - 'qual_select_one': QualSelectOneField, - 'qual_tags': QualTagsField, - 'qual_text': QualField, + 'qualAutoKeywordCount': QualField, + 'qualInteger': QualNumField, + 'qualNote': QualField, + 'qualSelectMultiple': QualSelectMultipleField, + 'qualSelectOne': QualSelectOneField, + 'qualTags': QualTagsField, + 'qualText': QualField, 'transcript': QualTranscriptField, 'translation': QualTranslationField, } @@ -530,20 +530,25 @@ def get_labels(self, *args, **kwargs): return [self._get_label(*args, **kwargs)] def get_value_from_entry(self, entry): - name = self.name.split('/')[-1] + name_parts = self.name.split('/') + # must have at least the source question path and the qual field uuid + assert len(name_parts) >= 2 + field_uuid = name_parts[-1] + # is it still necessary to have a separate `source` attribute? + source = '/'.join(name_parts[:-1]) + assert source == self.source try: - responses = entry['_supplementalDetails'][self.source_field.path][ - 'qual' - ] + # all responses nested within `qual` + responses = entry['_supplementalDetails'][source]['qual'] except KeyError: return '' # sure would be nice if this were a dict with uuids as keys instead of # a list requiring this kind of iteration for response in responses: - if response['uuid'] == name: - return response['val'] + if response['uuid'] == field_uuid: + return response['value'] return '' @@ -614,53 +619,61 @@ def get_value_from_entry(self, entry): return list_to_csv(val) -class QualTranscriptField(QualField): - def _get_label(self, *args, **kwargs): - source_label = self.source_field._get_label(*args, **kwargs) - return f'{source_label} - transcript ({self.language})' - +class QualNameSplittingTransxField(QualField): + """ + The (largely pre-refactor) structure of `_supplementalDetails` should be + changed to match the improved structure of `analysis_form`, and this `name` + splitting logic should be trashed. + """ def get_value_from_entry(self, entry): - name = self.name.split('/')[-1] + name_parts = self.name.split('/') + # must have at least the source question path and `transcript_??` or + # `translation_??` (where `??` is the language code) + assert len(name_parts) >= 2 + transx, _lang = name_parts[-1].split('_') # 🤢 + assert transx in ('transcript', 'translation') + assert _lang == self.language + # is it still necessary to have a separate `source` attribute? + source = '/'.join(name_parts[:-1]) + assert source == self.source try: - responses = entry['_supplementalDetails'][self.source_field.path] + responses = entry['_supplementalDetails'][source][transx] except KeyError: return '' - name_without_lang, lang = name.split('_') - assert name_without_lang == 'transcript' - + # A transcript does not have a dictionary with language keys: + # {'transcript': {'languageCode': 'en', 'value': 'i am a raisin'}} + if responses.get('languageCode') == self.language: + # Grab the value directly + return responses['value'] + + # However, translations do have an outer dictionary with language keys: + # { + # 'translation': { + # 'es': {'languageCode': 'es', 'value': 'soy una pasa'}, + # 'fr': { + # 'languageCode': 'fr', + # 'value': 'je suis un raisin sec', + # }, + # } + # } try: - response = responses['transcript'] + return responses[self.language]['value'] except KeyError: return '' - if response.get('languageCode') == lang: - return response['value'] - else: - return '' - -class QualTranslationField(QualField): +class QualTranscriptField(QualNameSplittingTransxField): def _get_label(self, *args, **kwargs): source_label = self.source_field._get_label(*args, **kwargs) - return f'{source_label} - translation ({self.language})' - - def get_value_from_entry(self, entry): - name = self.name.split('/')[-1] - - try: - responses = entry['_supplementalDetails'][self.source_field.path] - except KeyError: - return '' + return f'{source_label} - transcript ({self.language})' - name_without_lang, lang = name.split('_') - assert name_without_lang == 'translation' - try: - return responses['translation'][lang]['value'] - except KeyError: - return '' +class QualTranslationField(QualNameSplittingTransxField): + def _get_label(self, *args, **kwargs): + source_label = self.source_field._get_label(*args, **kwargs) + return f'{source_label} - translation ({self.language})' class MediaField(TextField): diff --git a/src/formpack/version.py b/src/formpack/version.py index 7a68a16a..f4b2e9b2 100644 --- a/src/formpack/version.py +++ b/src/formpack/version.py @@ -81,15 +81,13 @@ def __init__( formpack: 'FormPack', schema: Dict[str, Union[str, List]], ) -> None: - self.schema = schema self.formpack = formpack - survey = self.schema.get('additional_fields', []) - fields_by_name = self._get_fields_by_name(survey) + survey = self.schema section = FormSection(name=formpack.title) - self.translations = self._get_translations(schema) + self.translations = [UNTRANSLATED] for data_def in survey: field = FormField.from_json_definition( diff --git a/tests/fixtures/analysis_form/analysis_form.json b/tests/fixtures/analysis_form/analysis_form.json index 296dd9d1..7f1fb0fa 100644 --- a/tests/fixtures/analysis_form/analysis_form.json +++ b/tests/fixtures/analysis_form/analysis_form.json @@ -1,88 +1,38 @@ -{ - "engines": { - "engines/transcript_manual": { - "details": "A human provided transcription" - } +[ + { + "type": "transcript", + "name": "record_a_note/transcript_en", + "language": "en", + "source": "record_a_note" }, - "additional_fields": [ - { - "type": "transcript", - "name": "record_a_note/transcript_en", - "path": [ - "record_a_note", - "transcript_en" - ], - "language": "en", - "source": "record_a_note", - "settings": { - "mode": "manual", - "engine": "engines/transcript_manual" - } - }, - { - "type": "transcript", - "name": "record_a_note/transcript_es", - "path": [ - "record_a_note", - "transcript_es" - ], - "language": "es", - "source": "record_a_note", - "settings": { - "mode": "manual", - "engine": "engines/transcript_manual" - } - }, - { - "type": "translation", - "name": "record_a_note/translation_en", - "language": "en", - "path": [ - "record_a_note", - "translation_en" - ], - "source": "record_a_note", - "settings": { - "mode": "manual", - "engine": "engines/translation_manual" - } - }, - { - "type": "translation", - "name": "record_a_note/translation_es", - "language": "es", - "path": [ - "record_a_note", - "translation_es" - ], - "source": "record_a_note", - "settings": { - "mode": "manual", - "engine": "engines/translation_manual" - } - }, - { - "type": "qual_text", - "name": "name_of_clerk/comment", - "path": [ - "clerk_details/name_of_clerk", - "comment" - ], - "label": "Comment on the name of the clerk", - "source": "clerk_details/name_of_clerk" - }, - { - "type": "qual_text", - "name": "name_of_shop/comment", - "path": [ - "clerk_details/name_of_shop", - "comment" - ], - "label": "Comment on the name of the shop", - "source": "clerk_details/name_of_shop" - } - ], - "translations": [ - "English (en)" - ] -} + { + "type": "transcript", + "name": "record_a_note/transcript_es", + "language": "es", + "source": "record_a_note" + }, + { + "type": "translation", + "name": "record_a_note/translation_en", + "language": "en", + "source": "record_a_note" + }, + { + "type": "translation", + "name": "record_a_note/translation_es", + "language": "es", + "source": "record_a_note" + }, + { + "type": "qualText", + "name": "clerk_details/name_of_clerk/uuid_for_comment", + "label": "Comment on the name of the clerk", + "source": "clerk_details/name_of_clerk" + }, + { + "type": "qualText", + "name": "clerk_details/name_of_shop/uuid_for_comment", + "label": "Comment on the name of the shop", + "source": "clerk_details/name_of_shop" + } +] diff --git a/tests/fixtures/analysis_form/v1.json b/tests/fixtures/analysis_form/v1.json index fd0a0894..dac01cb4 100644 --- a/tests/fixtures/analysis_form/v1.json +++ b/tests/fixtures/analysis_form/v1.json @@ -59,15 +59,16 @@ }, "translation": { "en": { - "value": "Hello how may I help you?" + "value": "Hello how may I help you?", + "languageCode": "en" } } }, "clerk_details/name_of_clerk": { "qual": [ { - "uuid": "comment", - "val": "Sounds like an interesting person" + "uuid": "uuid_for_comment", + "value": "Sounds like an interesting person" } ] } diff --git a/tests/fixtures/analysis_form/v2.json b/tests/fixtures/analysis_form/v2.json index 6a8ade9f..cfd88774 100644 --- a/tests/fixtures/analysis_form/v2.json +++ b/tests/fixtures/analysis_form/v2.json @@ -61,8 +61,8 @@ "clerk_details/name_of_shop": { "qual": [ { - "uuid": "comment", - "val": "Pretty cliche" + "uuid": "uuid_for_comment", + "value": "Pretty cliche" } ] } diff --git a/tests/fixtures/analysis_form_advanced/analysis_form.json b/tests/fixtures/analysis_form_advanced/analysis_form.json index c98571ae..51eac802 100644 --- a/tests/fixtures/analysis_form_advanced/analysis_form.json +++ b/tests/fixtures/analysis_form_advanced/analysis_form.json @@ -1,121 +1,90 @@ -{ - "engines": { - "transcript": { - "details": "an external service provided by ACME, Inc." - } +[ + { + "type": "transcript", + "name": "clerk_interactions/record_a_note/transcript_en", + "source": "clerk_interactions/record_a_note", + "language": "en" }, - "additional_fields": [ - { - "type": "transcript", - "name": "record_a_note/transcript_en", - "path": [ - "clerk_interactions/record_a_note", - "transcript_en" - ], - "source": "clerk_interactions/record_a_note", - "language": "en", - "settings": { - "mode": "auto", - "engine": "engines/acme_1_speech2text" + { + "type": "qualSelectMultiple", + "name": "clerk_interactions/record_a_note/uuid_for_tone_of_voice", + "label": "How was the tone of the clerk's voice?", + "labels__for_future_use": [ + "How was the tone of the clerk's voice?", + "Kiel estis la tono de la voĉo de la oficisto?" + ], + "source": "clerk_interactions/record_a_note", + "choices": [ + { + "uuid": "uuid_for_anxious", + "labels": { + "_default": "Anxious", + "en": "Anxious", + "es": "Maltrankvila" + } + }, + { + "uuid": "uuid_for_excited", + "labels": { + "_default": "Excited", + "en": "Excited", + "es": "Ekscitita" + } + }, + { + "uuid": "uuid_for_confused", + "labels": { + "_default": "Confused", + "en": "Confused", + "es": "Konfuzita" + } } - }, - { - "type": "qual_select_multiple", - "name": "record_a_note/tone_of_voice", - "path": [ - "clerk_interactions/record_a_note", - "tone_of_voice" - ], - "label": "How was the tone of the clerk's voice?", - "labels__for_future_use": [ - "How was the tone of the clerk's voice?", - "Kiel estis la tono de la voĉo de la oficisto?" - ], - "source": "clerk_interactions/record_a_note", - "choices": [ - { - "uuid": "anxious", - "labels": { - "_default": "Anxious", - "en": "Anxious", - "es": "Maltrankvila" - } - }, - { - "uuid": "excited", - "labels": { - "_default": "Excited", - "en": "Excited", - "es": "Ekscitita" - } - }, - { - "uuid": "confused", - "labels": { - "_default": "Confused", - "en": "Confused", - "es": "Konfuzita" - } + ] + }, + { + "type": "qualText", + "name": "clerk_interactions/goods_sold/uuid_for_comment", + "label": "Comment on the goods sold at the store", + "labels__for_future_use": [ + "Comment on the goods sold at the store", + "Komentu la varojn venditajn en la vendejo" + ], + "source": "clerk_interactions/goods_sold" + }, + { + "type": "qualSelectOne", + "name": "clerk_interactions/goods_sold/uuid_for_rating", + "label": "Rate the quality of the goods sold at the store", + "labels__for_future_use": [ + "Rate the quality of the goods sold at the store", + "Komentu la varojn vendojn en la vendejo" + ], + "source": "clerk_interactions/goods_sold", + "choices": [ + { + "uuid": "uuid_for_1", + "labels": { + "_default": "Poor quality", + "en": "Poor quality", + "es": "Malbona kvalito" + } + }, + { + "uuid": "uuid_for_2", + "labels": { + "_default": "Average quality", + "en": "Average quality", + "es": "Meza kvalito" } - ] - }, - { - "type": "qual_text", - "name": "goods_sold/comment", - "path": [ - "clerk_interactions/goods_sold", - "comment" - ], - "label": "Comment on the goods sold at the store", - "labels__for_future_use": [ - "Comment on the goods sold at the store", - "Komentu la varojn venditajn en la vendejo" - ], - "source": "clerk_interactions/goods_sold" - }, - { - "type": "qual_select_one", - "name": "goods_sold/rating", - "path": [ - "clerk_interactions/goods_sold", - "rating" - ], - "label": "Rate the quality of the goods sold at the store", - "labels__for_future_use": [ - "Rate the quality of the goods sold at the store", - "Komentu la varojn vendojn en la vendejo" - ], - "source": "clerk_interactions/goods_sold", - "choices": [ - { - "uuid": "1", - "labels": { - "_default": "Poor quality", - "en": "Poor quality", - "es": "Malbona kvalito" - } - }, - { - "uuid": "2", - "labels": { - "_default": "Average quality", - "en": "Average quality", - "es": "Meza kvalito" - } - }, - { - "uuid": "3", - "labels": { - "_default": "High quality", - "en": "High quality", - "es": "Alta kvalito" - } + }, + { + "uuid": "uuid_for_3", + "labels": { + "_default": "High quality", + "en": "High quality", + "es": "Alta kvalito" } - ] - } - ], - "translations": [ - "English (en)", - "Esperanto (es)" - ] -} + } + ] + } +] diff --git a/tests/fixtures/analysis_form_advanced/v1.json b/tests/fixtures/analysis_form_advanced/v1.json index f56a5c21..42e9c285 100644 --- a/tests/fixtures/analysis_form_advanced/v1.json +++ b/tests/fixtures/analysis_form_advanced/v1.json @@ -85,15 +85,39 @@ }, "qual": [ { - "uuid": "tone_of_voice", - "val": [{"uuid": "excited"}, {"uuid": "confused"}] + "uuid": "uuid_for_tone_of_voice", + "value": [ + { + "uuid": "uuid_for_excited", + "labels": { + "_default": "Excited" + } + }, + { + "uuid": "uuid_for_confused", + "labels": { + "_default": "Confused" + } + } + ] } ] }, "clerk_interactions/goods_sold": { "qual": [ - {"uuid": "comment", "val": "Not much diversity"}, - {"uuid": "rating", "val": {"uuid": "3"}} + { + "uuid": "uuid_for_comment", + "value": "Not much diversity" + }, + { + "uuid": "uuid_for_rating", + "value": { + "uuid": "uuid_for_3", + "labels": { + "_default": "3" + } + } + } ] } } @@ -115,14 +139,35 @@ }, "qual": [ { - "uuid": "tone_of_voice", - "val": [{"uuid": "anxious"}, {"uuid": "excited"}] + "uuid": "uuid_for_tone_of_voice", + "value": [ + { + "uuid": "uuid_for_anxious", + "labels": { + "_default": "Anxious" + } + }, + { + "uuid": "uuid_for_excited", + "labels": { + "_default": "Excited" + } + } + ] } ] }, "clerk_interactions/goods_sold": { "qual": [ - {"uuid": "rating", "val": {"uuid": "2"}} + { + "uuid": "uuid_for_rating", + "value": { + "uuid": "uuid_for_2", + "labels": { + "_default": "2" + } + } + } ] } } @@ -139,7 +184,15 @@ "_supplementalDetails": { "clerk_interactions/goods_sold": { "qual": [ - {"uuid": "rating", "val": {"uuid": "3"}} + { + "uuid": "uuid_for_rating", + "value": { + "uuid": "uuid_for_3", + "labels": { + "_default": "3" + } + } + } ] } } diff --git a/tests/fixtures/analysis_form_repeat_groups/__init__.py b/tests/fixtures/analysis_form_repeat_groups/__init__.py index e484b9b2..1b6b542e 100644 --- a/tests/fixtures/analysis_form_repeat_groups/__init__.py +++ b/tests/fixtures/analysis_form_repeat_groups/__init__.py @@ -1,3 +1,5 @@ +# TODO: Figure out if this is ever even used (?) + # coding: utf-8 ''' analysis_form_repeat_groups diff --git a/tests/fixtures/analysis_form_repeat_groups/analysis_form.json b/tests/fixtures/analysis_form_repeat_groups/analysis_form.json index 8ccbe323..21c64c50 100644 --- a/tests/fixtures/analysis_form_repeat_groups/analysis_form.json +++ b/tests/fixtures/analysis_form_repeat_groups/analysis_form.json @@ -1,31 +1,19 @@ { - "engines": { - "transcript": { - "details": "an external service provided by ACME, Inc." - } - }, "additional_fields": [ { "type": "text", "name": "record_a_note/transcript", - "path": ["record_a_note", "transcript"], - "source": "record_a_note", - "analysis_type": "transcript", - "settings": { - "mode": "auto", - "engine": "engines/acme_1_speech2text" - } + "source": "record_a_note" }, { "type": "text", "name": "record_a_noise/comment_on_noise_level", - "path": ["record_a_noise", "comment_on_noise_level"], - "label": [ + "label": "Comment on noise level", + "labels__for_future_use": [ "Comment on noise level", "Komentu pri brunivelo" ], - "source": "record_a_noise", - "analysis_type": "coding" + "source": "record_a_noise" } ], "translations": [ diff --git a/tests/test_additional_field_exports.py b/tests/test_additional_field_exports.py index 08898129..2fe8650a 100644 --- a/tests/test_additional_field_exports.py +++ b/tests/test_additional_field_exports.py @@ -321,6 +321,7 @@ def test_additional_field_exports_v2(): 'record_a_note - transcript (es)', 'record_a_note - translation (en)', 'record_a_note - translation (es)', + # `name_of_clerk` is absent in v2 'name_of_shop', 'name_of_shop - Comment on the name of the shop', ] @@ -395,6 +396,9 @@ def test_additional_field_exports_all_versions_exclude_fields(): 'versions': pack.versions, 'filter_fields': [ 'record_a_note', + # FIXME: These make no sense because `name_of_*` are regular survey questions, not "additional fields" + # Or is the idea that by selecting the source fields the related additional fields come along automatically? + # But if that's true, why do we explicitly request the additional fields in `test_additional_field_exports_with_labels()`? '_supplementalDetails/clerk_details/name_of_shop', '_supplementalDetails/clerk_details/name_of_clerk', ], diff --git a/tests/test_fixtures_valid.py b/tests/test_fixtures_valid.py index 0a6a4348..6d756dec 100644 --- a/tests/test_fixtures_valid.py +++ b/tests/test_fixtures_valid.py @@ -132,18 +132,25 @@ def test_analysis_form(self): assert 'Simple Clerk Interaction' == title expected_analysis_questions = sorted( - [f['name'] for f in analysis_form['additional_fields']] + ( + 'record_a_note/transcript_en', + 'record_a_note/transcript_es', + 'record_a_note/translation_en', + 'record_a_note/translation_es', + 'clerk_details/name_of_clerk/uuid_for_comment', + 'clerk_details/name_of_shop/uuid_for_comment', + ) ) actual_analysis_questions = sorted( [f.name for f in fp.analysis_form.fields] ) assert expected_analysis_questions == actual_analysis_questions - f1 = fp.analysis_form.fields[0] - assert hasattr(f1, 'source') and f1.source - assert hasattr(f1, 'has_stats') and not f1.has_stats - assert hasattr(f1, 'settings') - assert f1.data_type in ( - 'transcript', - 'translation', - ) or f1.data_type.startswith('qual_') + for field in fp.analysis_form.fields: + assert hasattr(field, 'source') and field.source + assert hasattr(field, 'has_stats') and not field.has_stats + assert hasattr(field, 'settings') + assert field.data_type in ( + 'transcript', + 'translation', + ) or field.data_type.startswith('qual')