From 75358c55336a7a6aef881ee76d3a427219125a0e Mon Sep 17 00:00:00 2001 From: "John N. Milner" Date: Fri, 22 Aug 2025 23:57:18 -0400 Subject: [PATCH 1/4] Remove unused `analysis_form` fixture data --- .../fixtures/analysis_form/analysis_form.json | 53 ++----------------- .../analysis_form_advanced/analysis_form.json | 27 +--------- .../analysis_form.json | 20 ++----- 3 files changed, 9 insertions(+), 91 deletions(-) diff --git a/tests/fixtures/analysis_form/analysis_form.json b/tests/fixtures/analysis_form/analysis_form.json index 296dd9d1..5dce23f0 100644 --- a/tests/fixtures/analysis_form/analysis_form.json +++ b/tests/fixtures/analysis_form/analysis_form.json @@ -1,83 +1,38 @@ { - "engines": { - "engines/transcript_manual": { - "details": "A human provided transcription" - } - }, "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" - } + "source": "record_a_note" }, { "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" - } + "source": "record_a_note" }, { "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" - } + "source": "record_a_note" }, { "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" - } + "source": "record_a_note" }, { "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" } diff --git a/tests/fixtures/analysis_form_advanced/analysis_form.json b/tests/fixtures/analysis_form_advanced/analysis_form.json index c98571ae..39effd4a 100644 --- a/tests/fixtures/analysis_form_advanced/analysis_form.json +++ b/tests/fixtures/analysis_form_advanced/analysis_form.json @@ -1,31 +1,14 @@ { - "engines": { - "transcript": { - "details": "an external service provided by ACME, Inc." - } - }, "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" - } + "language": "en" }, { "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?", @@ -62,10 +45,6 @@ { "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", @@ -76,10 +55,6 @@ { "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", 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": [ From 0da20e9699aa33481fb6e685af76d43dce8a80c5 Mon Sep 17 00:00:00 2001 From: "John N. Milner" Date: Mon, 25 Aug 2025 01:48:42 -0400 Subject: [PATCH 2/4] =?UTF-8?q?Simplify=20format=20of=20submission=20suppl?= =?UTF-8?q?ements;=20in=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit anticipation of matching changes to kobotoolbox/kpi --- src/formpack/schema/fields.py | 29 +-- src/formpack/version.py | 6 +- .../fixtures/analysis_form/analysis_form.json | 81 ++++---- tests/fixtures/analysis_form/v1.json | 25 +-- tests/fixtures/analysis_form/v2.json | 16 +- .../analysis_form_advanced/analysis_form.json | 182 +++++++++--------- tests/fixtures/analysis_form_advanced/v1.json | 16 +- tests/test_additional_field_exports.py | 16 +- tests/test_fixtures_valid.py | 9 +- 9 files changed, 170 insertions(+), 210 deletions(-) diff --git a/src/formpack/schema/fields.py b/src/formpack/schema/fields.py index 428c0577..19e3cedf 100644 --- a/src/formpack/schema/fields.py +++ b/src/formpack/schema/fields.py @@ -620,26 +620,11 @@ def _get_label(self, *args, **kwargs): return f'{source_label} - transcript ({self.language})' def get_value_from_entry(self, entry): - name = self.name.split('/')[-1] - try: - responses = entry['_supplementalDetails'][self.source_field.path] + return entry['_supplementalDetails'][self.name]['value'] except KeyError: return '' - name_without_lang, lang = name.split('_') - assert name_without_lang == 'transcript' - - try: - response = responses['transcript'] - except KeyError: - return '' - - if response.get('languageCode') == lang: - return response['value'] - else: - return '' - class QualTranslationField(QualField): def _get_label(self, *args, **kwargs): @@ -647,18 +632,8 @@ def _get_label(self, *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 '' - - name_without_lang, lang = name.split('_') - assert name_without_lang == 'translation' - try: - return responses['translation'][lang]['value'] + return entry['_supplementalDetails'][self.name]['value'] except KeyError: return '' 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 5dce23f0..61ef2c57 100644 --- a/tests/fixtures/analysis_form/analysis_form.json +++ b/tests/fixtures/analysis_form/analysis_form.json @@ -1,43 +1,38 @@ -{ - "additional_fields": [ - { - "type": "transcript", - "name": "record_a_note/transcript_en", - "language": "en", - "source": "record_a_note" - }, - { - "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": "qual_text", - "name": "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", - "label": "Comment on the name of the shop", - "source": "clerk_details/name_of_shop" - } - ], - "translations": [ - "English (en)" - ] -} +[ + { + "type": "transcript", + "name": "record_a_note/transcript__en", + "language": "en", + "source": "record_a_note" + }, + { + "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": "qual_text", + "name": "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", + "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..7e6152f2 100644 --- a/tests/fixtures/analysis_form/v1.json +++ b/tests/fixtures/analysis_form/v1.json @@ -52,16 +52,13 @@ } ], "_supplementalDetails": { - "record_a_note": { - "transcript": { - "value": "Saluton, kiel mi povas helpi vin?", - "languageCode": "es" - }, - "translation": { - "en": { - "value": "Hello how may I help you?" - } - } + "record_a_note/transcript__es": { + "value": "Saluton, kiel mi povas helpi vin?", + "language": "es" + }, + "record_a_note/translation__en": { + "value": "Hello how may I help you?", + "language": "en" }, "clerk_details/name_of_clerk": { "qual": [ @@ -83,11 +80,9 @@ } ], "_supplementalDetails": { - "record_a_note": { - "transcript": { - "value": "Thank you for your business", - "languageCode": "en" - } + "record_a_note/transcript__en": { + "value": "Thank you for your business", + "language": "en" } } }, diff --git a/tests/fixtures/analysis_form/v2.json b/tests/fixtures/analysis_form/v2.json index 6a8ade9f..64b1d350 100644 --- a/tests/fixtures/analysis_form/v2.json +++ b/tests/fixtures/analysis_form/v2.json @@ -52,11 +52,9 @@ } ], "_supplementalDetails": { - "record_a_note": { - "transcript": { - "value": "Hello how may I help you?", - "languageCode": "en" - } + "record_a_note/transcript__en": { + "value": "Hello how may I help you?", + "language": "en" }, "clerk_details/name_of_shop": { "qual": [ @@ -78,11 +76,9 @@ } ], "_supplementalDetails": { - "record_a_note": { - "transcript": { - "value": "Thank you for your business", - "languageCode": "en" - } + "record_a_note/transcript__en": { + "value": "Thank you for your business", + "language": "en" } } }, diff --git a/tests/fixtures/analysis_form_advanced/analysis_form.json b/tests/fixtures/analysis_form_advanced/analysis_form.json index 39effd4a..32b62bf0 100644 --- a/tests/fixtures/analysis_form_advanced/analysis_form.json +++ b/tests/fixtures/analysis_form_advanced/analysis_form.json @@ -1,96 +1,90 @@ -{ - "additional_fields": [ - { - "type": "transcript", - "name": "record_a_note/transcript_en", - "source": "clerk_interactions/record_a_note", - "language": "en" - }, - { - "type": "qual_select_multiple", - "name": "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": "transcript", + "name": "clerk_interactions/record_a_note/transcript__en", + "source": "clerk_interactions/record_a_note", + "language": "en" + }, + { + "type": "qual_select_multiple", + "name": "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" } - ] - }, - { - "type": "qual_text", - "name": "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", - "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": "excited", + "labels": { + "_default": "Excited", + "en": "Excited", + "es": "Ekscitita" } - ] - } - ], - "translations": [ - "English (en)", - "Esperanto (es)" - ] -} + }, + { + "uuid": "confused", + "labels": { + "_default": "Confused", + "en": "Confused", + "es": "Konfuzita" + } + } + ] + }, + { + "type": "qual_text", + "name": "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", + "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" + } + } + ] + } +] diff --git a/tests/fixtures/analysis_form_advanced/v1.json b/tests/fixtures/analysis_form_advanced/v1.json index f56a5c21..9c21c241 100644 --- a/tests/fixtures/analysis_form_advanced/v1.json +++ b/tests/fixtures/analysis_form_advanced/v1.json @@ -78,11 +78,11 @@ } ], "_supplementalDetails": { + "clerk_interactions/record_a_note/transcript__en": { + "value": "Hello how may I help you?", + "language": "en" + }, "clerk_interactions/record_a_note": { - "transcript": { - "value": "Hello how may I help you?", - "languageCode": "en" - }, "qual": [ { "uuid": "tone_of_voice", @@ -108,11 +108,11 @@ } ], "_supplementalDetails": { + "clerk_interactions/record_a_note/transcript__en": { + "value": "Thank you for your business", + "language": "en" + }, "clerk_interactions/record_a_note": { - "transcript": { - "value": "Thank you for your business", - "languageCode": "en" - }, "qual": [ { "uuid": "tone_of_voice", diff --git a/tests/test_additional_field_exports.py b/tests/test_additional_field_exports.py index 08898129..ba6026cf 100644 --- a/tests/test_additional_field_exports.py +++ b/tests/test_additional_field_exports.py @@ -15,10 +15,10 @@ def test_additional_field_exports_without_labels(): 'versions': 'v1', 'filter_fields': [ 'record_a_note', - '_supplementalDetails/record_a_note/transcript_en', - '_supplementalDetails/record_a_note/transcript_es', - '_supplementalDetails/record_a_note/translation_en', - '_supplementalDetails/record_a_note/translation_es', + '_supplementalDetails/record_a_note/transcript__en', + '_supplementalDetails/record_a_note/transcript__es', + '_supplementalDetails/record_a_note/translation__en', + '_supplementalDetails/record_a_note/translation__es', ], } export = pack.export(**options) @@ -53,10 +53,10 @@ def test_additional_field_exports_with_labels(): 'versions': 'v1', 'filter_fields': [ 'record_a_note', - '_supplementalDetails/record_a_note/transcript_en', - '_supplementalDetails/record_a_note/transcript_es', - '_supplementalDetails/record_a_note/translation_en', - '_supplementalDetails/record_a_note/translation_es', + '_supplementalDetails/record_a_note/transcript__en', + '_supplementalDetails/record_a_note/transcript__es', + '_supplementalDetails/record_a_note/translation__en', + '_supplementalDetails/record_a_note/translation__es', ], 'lang': 'English (en)', } diff --git a/tests/test_fixtures_valid.py b/tests/test_fixtures_valid.py index 0a6a4348..dcf20d5f 100644 --- a/tests/test_fixtures_valid.py +++ b/tests/test_fixtures_valid.py @@ -132,7 +132,14 @@ 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', + 'name_of_clerk/comment', + 'name_of_shop/comment', + ) ) actual_analysis_questions = sorted( [f.name for f in fp.analysis_form.fields] From 5202d48bf2a9c128e146ddf3ab97100a4fc90a1c Mon Sep 17 00:00:00 2001 From: "John N. Milner" Date: Mon, 25 Aug 2025 10:29:37 -0400 Subject: [PATCH 3/4] Simplify the shape of qualitative analysis fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …following the new shape of transcripts and translations --- src/formpack/schema/fields.py | 15 ++++-- .../fixtures/analysis_form/analysis_form.json | 4 +- tests/fixtures/analysis_form/v1.json | 14 +++-- tests/fixtures/analysis_form/v2.json | 14 +++-- .../analysis_form_advanced/analysis_form.json | 6 +-- tests/fixtures/analysis_form_advanced/v1.json | 54 ++++++++----------- tests/test_fixtures_valid.py | 4 +- 7 files changed, 51 insertions(+), 60 deletions(-) diff --git a/src/formpack/schema/fields.py b/src/formpack/schema/fields.py index 19e3cedf..0220751e 100644 --- a/src/formpack/schema/fields.py +++ b/src/formpack/schema/fields.py @@ -530,19 +530,24 @@ 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 + response_key = source + '/qual' # all responses nested within this try: - responses = entry['_supplementalDetails'][self.source_field.path][ - 'qual' - ] + responses = entry['_supplementalDetails'][response_key] 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: + if response['uuid'] == field_uuid: return response['val'] return '' diff --git a/tests/fixtures/analysis_form/analysis_form.json b/tests/fixtures/analysis_form/analysis_form.json index 61ef2c57..b80d12ae 100644 --- a/tests/fixtures/analysis_form/analysis_form.json +++ b/tests/fixtures/analysis_form/analysis_form.json @@ -25,13 +25,13 @@ }, { "type": "qual_text", - "name": "name_of_clerk/comment", + "name": "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", + "name": "clerk_details/name_of_shop/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 7e6152f2..a00aa25f 100644 --- a/tests/fixtures/analysis_form/v1.json +++ b/tests/fixtures/analysis_form/v1.json @@ -60,14 +60,12 @@ "value": "Hello how may I help you?", "language": "en" }, - "clerk_details/name_of_clerk": { - "qual": [ - { - "uuid": "comment", - "val": "Sounds like an interesting person" - } - ] - } + "clerk_details/name_of_clerk/qual": [ + { + "uuid": "comment", + "val": "Sounds like an interesting person" + } + ] } }, { diff --git a/tests/fixtures/analysis_form/v2.json b/tests/fixtures/analysis_form/v2.json index 64b1d350..b88d4427 100644 --- a/tests/fixtures/analysis_form/v2.json +++ b/tests/fixtures/analysis_form/v2.json @@ -56,14 +56,12 @@ "value": "Hello how may I help you?", "language": "en" }, - "clerk_details/name_of_shop": { - "qual": [ - { - "uuid": "comment", - "val": "Pretty cliche" - } - ] - } + "clerk_details/name_of_shop/qual": [ + { + "uuid": "comment", + "val": "Pretty cliche" + } + ] } }, { diff --git a/tests/fixtures/analysis_form_advanced/analysis_form.json b/tests/fixtures/analysis_form_advanced/analysis_form.json index 32b62bf0..a6ce9f8c 100644 --- a/tests/fixtures/analysis_form_advanced/analysis_form.json +++ b/tests/fixtures/analysis_form_advanced/analysis_form.json @@ -7,7 +7,7 @@ }, { "type": "qual_select_multiple", - "name": "record_a_note/tone_of_voice", + "name": "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?", @@ -43,7 +43,7 @@ }, { "type": "qual_text", - "name": "goods_sold/comment", + "name": "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", @@ -53,7 +53,7 @@ }, { "type": "qual_select_one", - "name": "goods_sold/rating", + "name": "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", diff --git a/tests/fixtures/analysis_form_advanced/v1.json b/tests/fixtures/analysis_form_advanced/v1.json index 9c21c241..7b3dc41c 100644 --- a/tests/fixtures/analysis_form_advanced/v1.json +++ b/tests/fixtures/analysis_form_advanced/v1.json @@ -82,20 +82,16 @@ "value": "Hello how may I help you?", "language": "en" }, - "clerk_interactions/record_a_note": { - "qual": [ - { - "uuid": "tone_of_voice", - "val": [{"uuid": "excited"}, {"uuid": "confused"}] - } - ] - }, - "clerk_interactions/goods_sold": { - "qual": [ - {"uuid": "comment", "val": "Not much diversity"}, - {"uuid": "rating", "val": {"uuid": "3"}} - ] - } + "clerk_interactions/record_a_note/qual": [ + { + "uuid": "tone_of_voice", + "val": [{"uuid": "excited"}, {"uuid": "confused"}] + } + ], + "clerk_interactions/goods_sold/qual": [ + {"uuid": "comment", "val": "Not much diversity"}, + {"uuid": "rating", "val": {"uuid": "3"}} + ] } }, { @@ -112,19 +108,15 @@ "value": "Thank you for your business", "language": "en" }, - "clerk_interactions/record_a_note": { - "qual": [ - { - "uuid": "tone_of_voice", - "val": [{"uuid": "anxious"}, {"uuid": "excited"}] - } - ] - }, - "clerk_interactions/goods_sold": { - "qual": [ - {"uuid": "rating", "val": {"uuid": "2"}} - ] - } + "clerk_interactions/record_a_note/qual": [ + { + "uuid": "tone_of_voice", + "val": [{"uuid": "anxious"}, {"uuid": "excited"}] + } + ], + "clerk_interactions/goods_sold/qual": [ + {"uuid": "rating", "val": {"uuid": "2"}} + ] } }, { @@ -137,11 +129,9 @@ } ], "_supplementalDetails": { - "clerk_interactions/goods_sold": { - "qual": [ - {"uuid": "rating", "val": {"uuid": "3"}} - ] - } + "clerk_interactions/goods_sold/qual": [ + {"uuid": "rating", "val": {"uuid": "3"}} + ] } } ] diff --git a/tests/test_fixtures_valid.py b/tests/test_fixtures_valid.py index dcf20d5f..ad7f0aa3 100644 --- a/tests/test_fixtures_valid.py +++ b/tests/test_fixtures_valid.py @@ -137,8 +137,8 @@ def test_analysis_form(self): 'record_a_note/transcript__es', 'record_a_note/translation__en', 'record_a_note/translation__es', - 'name_of_clerk/comment', - 'name_of_shop/comment', + 'clerk_details/name_of_clerk/comment', + 'clerk_details/name_of_shop/comment', ) ) actual_analysis_questions = sorted( From eabc4559a0bc5c9c11d684b5ba14b31c636fd1ad Mon Sep 17 00:00:00 2001 From: "John N. Milner" Date: Mon, 15 Dec 2025 01:14:01 -0500 Subject: [PATCH 4/4] Match current state of kpi subsequences refactor --- src/formpack/schema/fields.py | 79 ++++++++---- .../fixtures/analysis_form/analysis_form.json | 16 +-- tests/fixtures/analysis_form/v1.json | 40 +++--- tests/fixtures/analysis_form/v2.json | 30 +++-- .../analysis_form_advanced/analysis_form.json | 26 ++-- tests/fixtures/analysis_form_advanced/v1.json | 119 +++++++++++++----- .../analysis_form_repeat_groups/__init__.py | 2 + tests/test_additional_field_exports.py | 20 +-- tests/test_fixtures_valid.py | 28 ++--- 9 files changed, 238 insertions(+), 122 deletions(-) diff --git a/src/formpack/schema/fields.py b/src/formpack/schema/fields.py index 0220751e..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, } @@ -537,10 +537,10 @@ def get_value_from_entry(self, entry): # is it still necessary to have a separate `source` attribute? source = '/'.join(name_parts[:-1]) assert source == self.source - response_key = source + '/qual' # all responses nested within this try: - responses = entry['_supplementalDetails'][response_key] + # all responses nested within `qual` + responses = entry['_supplementalDetails'][source]['qual'] except KeyError: return '' @@ -548,7 +548,7 @@ def get_value_from_entry(self, entry): # a list requiring this kind of iteration for response in responses: if response['uuid'] == field_uuid: - return response['val'] + return response['value'] return '' @@ -619,28 +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_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'][source][transx] + except KeyError: + return '' + + # 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: - return entry['_supplementalDetails'][self.name]['value'] + return responses[self.language]['value'] except KeyError: 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})' + return f'{source_label} - transcript ({self.language})' - def get_value_from_entry(self, entry): - try: - return entry['_supplementalDetails'][self.name]['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/tests/fixtures/analysis_form/analysis_form.json b/tests/fixtures/analysis_form/analysis_form.json index b80d12ae..7f1fb0fa 100644 --- a/tests/fixtures/analysis_form/analysis_form.json +++ b/tests/fixtures/analysis_form/analysis_form.json @@ -1,37 +1,37 @@ [ { "type": "transcript", - "name": "record_a_note/transcript__en", + "name": "record_a_note/transcript_en", "language": "en", "source": "record_a_note" }, { "type": "transcript", - "name": "record_a_note/transcript__es", + "name": "record_a_note/transcript_es", "language": "es", "source": "record_a_note" }, { "type": "translation", - "name": "record_a_note/translation__en", + "name": "record_a_note/translation_en", "language": "en", "source": "record_a_note" }, { "type": "translation", - "name": "record_a_note/translation__es", + "name": "record_a_note/translation_es", "language": "es", "source": "record_a_note" }, { - "type": "qual_text", - "name": "clerk_details/name_of_clerk/comment", + "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": "qual_text", - "name": "clerk_details/name_of_shop/comment", + "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 a00aa25f..dac01cb4 100644 --- a/tests/fixtures/analysis_form/v1.json +++ b/tests/fixtures/analysis_form/v1.json @@ -52,20 +52,26 @@ } ], "_supplementalDetails": { - "record_a_note/transcript__es": { - "value": "Saluton, kiel mi povas helpi vin?", - "language": "es" - }, - "record_a_note/translation__en": { - "value": "Hello how may I help you?", - "language": "en" - }, - "clerk_details/name_of_clerk/qual": [ - { - "uuid": "comment", - "val": "Sounds like an interesting person" + "record_a_note": { + "transcript": { + "value": "Saluton, kiel mi povas helpi vin?", + "languageCode": "es" + }, + "translation": { + "en": { + "value": "Hello how may I help you?", + "languageCode": "en" + } } - ] + }, + "clerk_details/name_of_clerk": { + "qual": [ + { + "uuid": "uuid_for_comment", + "value": "Sounds like an interesting person" + } + ] + } } }, { @@ -78,9 +84,11 @@ } ], "_supplementalDetails": { - "record_a_note/transcript__en": { - "value": "Thank you for your business", - "language": "en" + "record_a_note": { + "transcript": { + "value": "Thank you for your business", + "languageCode": "en" + } } } }, diff --git a/tests/fixtures/analysis_form/v2.json b/tests/fixtures/analysis_form/v2.json index b88d4427..cfd88774 100644 --- a/tests/fixtures/analysis_form/v2.json +++ b/tests/fixtures/analysis_form/v2.json @@ -52,16 +52,20 @@ } ], "_supplementalDetails": { - "record_a_note/transcript__en": { - "value": "Hello how may I help you?", - "language": "en" - }, - "clerk_details/name_of_shop/qual": [ - { - "uuid": "comment", - "val": "Pretty cliche" + "record_a_note": { + "transcript": { + "value": "Hello how may I help you?", + "languageCode": "en" } - ] + }, + "clerk_details/name_of_shop": { + "qual": [ + { + "uuid": "uuid_for_comment", + "value": "Pretty cliche" + } + ] + } } }, { @@ -74,9 +78,11 @@ } ], "_supplementalDetails": { - "record_a_note/transcript__en": { - "value": "Thank you for your business", - "language": "en" + "record_a_note": { + "transcript": { + "value": "Thank you for your business", + "languageCode": "en" + } } } }, diff --git a/tests/fixtures/analysis_form_advanced/analysis_form.json b/tests/fixtures/analysis_form_advanced/analysis_form.json index a6ce9f8c..51eac802 100644 --- a/tests/fixtures/analysis_form_advanced/analysis_form.json +++ b/tests/fixtures/analysis_form_advanced/analysis_form.json @@ -1,13 +1,13 @@ [ { "type": "transcript", - "name": "clerk_interactions/record_a_note/transcript__en", + "name": "clerk_interactions/record_a_note/transcript_en", "source": "clerk_interactions/record_a_note", "language": "en" }, { - "type": "qual_select_multiple", - "name": "clerk_interactions/record_a_note/tone_of_voice", + "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?", @@ -16,7 +16,7 @@ "source": "clerk_interactions/record_a_note", "choices": [ { - "uuid": "anxious", + "uuid": "uuid_for_anxious", "labels": { "_default": "Anxious", "en": "Anxious", @@ -24,7 +24,7 @@ } }, { - "uuid": "excited", + "uuid": "uuid_for_excited", "labels": { "_default": "Excited", "en": "Excited", @@ -32,7 +32,7 @@ } }, { - "uuid": "confused", + "uuid": "uuid_for_confused", "labels": { "_default": "Confused", "en": "Confused", @@ -42,8 +42,8 @@ ] }, { - "type": "qual_text", - "name": "clerk_interactions/goods_sold/comment", + "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", @@ -52,8 +52,8 @@ "source": "clerk_interactions/goods_sold" }, { - "type": "qual_select_one", - "name": "clerk_interactions/goods_sold/rating", + "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", @@ -62,7 +62,7 @@ "source": "clerk_interactions/goods_sold", "choices": [ { - "uuid": "1", + "uuid": "uuid_for_1", "labels": { "_default": "Poor quality", "en": "Poor quality", @@ -70,7 +70,7 @@ } }, { - "uuid": "2", + "uuid": "uuid_for_2", "labels": { "_default": "Average quality", "en": "Average quality", @@ -78,7 +78,7 @@ } }, { - "uuid": "3", + "uuid": "uuid_for_3", "labels": { "_default": "High quality", "en": "High quality", diff --git a/tests/fixtures/analysis_form_advanced/v1.json b/tests/fixtures/analysis_form_advanced/v1.json index 7b3dc41c..42e9c285 100644 --- a/tests/fixtures/analysis_form_advanced/v1.json +++ b/tests/fixtures/analysis_form_advanced/v1.json @@ -78,20 +78,48 @@ } ], "_supplementalDetails": { - "clerk_interactions/record_a_note/transcript__en": { - "value": "Hello how may I help you?", - "language": "en" + "clerk_interactions/record_a_note": { + "transcript": { + "value": "Hello how may I help you?", + "languageCode": "en" + }, + "qual": [ + { + "uuid": "uuid_for_tone_of_voice", + "value": [ + { + "uuid": "uuid_for_excited", + "labels": { + "_default": "Excited" + } + }, + { + "uuid": "uuid_for_confused", + "labels": { + "_default": "Confused" + } + } + ] + } + ] }, - "clerk_interactions/record_a_note/qual": [ - { - "uuid": "tone_of_voice", - "val": [{"uuid": "excited"}, {"uuid": "confused"}] - } - ], - "clerk_interactions/goods_sold/qual": [ - {"uuid": "comment", "val": "Not much diversity"}, - {"uuid": "rating", "val": {"uuid": "3"}} - ] + "clerk_interactions/goods_sold": { + "qual": [ + { + "uuid": "uuid_for_comment", + "value": "Not much diversity" + }, + { + "uuid": "uuid_for_rating", + "value": { + "uuid": "uuid_for_3", + "labels": { + "_default": "3" + } + } + } + ] + } } }, { @@ -104,19 +132,44 @@ } ], "_supplementalDetails": { - "clerk_interactions/record_a_note/transcript__en": { - "value": "Thank you for your business", - "language": "en" + "clerk_interactions/record_a_note": { + "transcript": { + "value": "Thank you for your business", + "languageCode": "en" + }, + "qual": [ + { + "uuid": "uuid_for_tone_of_voice", + "value": [ + { + "uuid": "uuid_for_anxious", + "labels": { + "_default": "Anxious" + } + }, + { + "uuid": "uuid_for_excited", + "labels": { + "_default": "Excited" + } + } + ] + } + ] }, - "clerk_interactions/record_a_note/qual": [ - { - "uuid": "tone_of_voice", - "val": [{"uuid": "anxious"}, {"uuid": "excited"}] - } - ], - "clerk_interactions/goods_sold/qual": [ - {"uuid": "rating", "val": {"uuid": "2"}} - ] + "clerk_interactions/goods_sold": { + "qual": [ + { + "uuid": "uuid_for_rating", + "value": { + "uuid": "uuid_for_2", + "labels": { + "_default": "2" + } + } + } + ] + } } }, { @@ -129,9 +182,19 @@ } ], "_supplementalDetails": { - "clerk_interactions/goods_sold/qual": [ - {"uuid": "rating", "val": {"uuid": "3"}} - ] + "clerk_interactions/goods_sold": { + "qual": [ + { + "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/test_additional_field_exports.py b/tests/test_additional_field_exports.py index ba6026cf..2fe8650a 100644 --- a/tests/test_additional_field_exports.py +++ b/tests/test_additional_field_exports.py @@ -15,10 +15,10 @@ def test_additional_field_exports_without_labels(): 'versions': 'v1', 'filter_fields': [ 'record_a_note', - '_supplementalDetails/record_a_note/transcript__en', - '_supplementalDetails/record_a_note/transcript__es', - '_supplementalDetails/record_a_note/translation__en', - '_supplementalDetails/record_a_note/translation__es', + '_supplementalDetails/record_a_note/transcript_en', + '_supplementalDetails/record_a_note/transcript_es', + '_supplementalDetails/record_a_note/translation_en', + '_supplementalDetails/record_a_note/translation_es', ], } export = pack.export(**options) @@ -53,10 +53,10 @@ def test_additional_field_exports_with_labels(): 'versions': 'v1', 'filter_fields': [ 'record_a_note', - '_supplementalDetails/record_a_note/transcript__en', - '_supplementalDetails/record_a_note/transcript__es', - '_supplementalDetails/record_a_note/translation__en', - '_supplementalDetails/record_a_note/translation__es', + '_supplementalDetails/record_a_note/transcript_en', + '_supplementalDetails/record_a_note/transcript_es', + '_supplementalDetails/record_a_note/translation_en', + '_supplementalDetails/record_a_note/translation_es', ], 'lang': 'English (en)', } @@ -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 ad7f0aa3..6d756dec 100644 --- a/tests/test_fixtures_valid.py +++ b/tests/test_fixtures_valid.py @@ -133,12 +133,12 @@ def test_analysis_form(self): expected_analysis_questions = sorted( ( - '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/comment', - 'clerk_details/name_of_shop/comment', + '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( @@ -146,11 +146,11 @@ def test_analysis_form(self): ) 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')