Fix bug in Reshape::FieldsToFieldGroupWithConstant

This was triggered only when the length of the given fieldmap was 1
lyrasis · Nov 21, 2023 · fe6944e · fe6944e
1 parent c6517ff
commit fe6944e
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 22 deletions.
diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc
@@ -34,6 +34,7 @@ These changes are merged into the `main` branch, but have not been released. Aft
 
 * `IterativeCleanup` now automatically extends its extending module with `Dry::Configurable` prior to defining settings that depend on `Dry::Configurable`. (PR#192)
 * `Kiba::Extend::Job.output?` no longer fails if given job returns Nil (PR#194)
+* `Reshape::FieldsToFieldGroupWithConstant` constant value is no longer added to rows with no values in the renamed/remapped value fields, when fieldmap length == 1. (PR#195)
 
 === Added
 

diff --git a/lib/kiba/extend/transforms/reshape/fields_to_field_group_with_constant.rb b/lib/kiba/extend/transforms/reshape/fields_to_field_group_with_constant.rb
@@ -195,9 +195,15 @@ class FieldsToFieldGroupWithConstant
           # @param uneven_warning [Boolean] whether to print warnings about uneven field groups to STDOUT
           # @param remove_empty_groups [Boolean] whether to run {Delete::EmptyFieldGroups} before finalizing row
           # @param delim [String] used to split/join multiple values in a field
-          def initialize(fieldmap:, constant_target:, constant_value:, delim: Kiba::Extend.delim,
-            replace_empty: true, treat_as_null: Kiba::Extend.nullvalue,
-            enforce_evenness: true, evener: nil, uneven_warning: true,
+          def initialize(fieldmap:,
+            constant_target:,
+            constant_value:,
+            delim: Kiba::Extend.delim,
+            replace_empty: true,
+            treat_as_null: Kiba::Extend.nullvalue,
+            enforce_evenness: true,
+            evener: nil,
+            uneven_warning: true,
             remove_empty_groups: true)
             @renamer = Rename::Fields.new(fieldmap: fieldmap)
             @renamed = fieldmap.values
@@ -211,26 +217,44 @@ def initialize(fieldmap:, constant_target:, constant_value:, delim: Kiba::Extend
             @uneven_warning = uneven_warning
             @remove_empty_groups = remove_empty_groups
 
-            @empty_replacer = Replace::EmptyFieldValues.new(fields: @renamed, value: treat_as_null, delim: delim,
-              treat_as_null: treat_as_null)
-            @even_xform = Clean::EvenFieldValues.new(fields: @renamed, evener: @evener, delim: delim,
-              warn: uneven_warning)
+            @empty_replacer = Replace::EmptyFieldValues.new(
+              fields: @renamed,
+              value: treat_as_null,
+              delim: delim,
+              treat_as_null: treat_as_null
+            )
+            @even_xform = Clean::EvenFieldValues.new(
+              fields: @renamed,
+              evener: @evener,
+              delim: delim,
+              warn: uneven_warning
+            )
             @group_cleaner = Delete::EmptyFieldGroups.new(
-              groups: [[renamed,
-                target].flatten], delim: delim, treat_as_null: treat_as_null
+              groups: [[renamed, target].flatten],
+              delim: delim,
+              treat_as_null: treat_as_null
+            )
+            @value_getter = Helpers::FieldValueGetter.new(
+              fields: renamed,
+              delim: delim,
+              treat_as_null: treat_as_null
             )
-            @value_getter = Helpers::FieldValueGetter.new(fields: renamed,
-              delim: delim, treat_as_null: treat_as_null)
           end
 
           # @param row [Hash{ Symbol => String, nil }]
           def process(row)
             renamer.process(row)
-            empty_replacer.process(row) if replace_empty
-            max_vals = find_max_vals(row)
-            add_constant(row, max_vals)
-            even_xform.process(row) if enforce_evenness
-            group_cleaner.process(row) if remove_empty_groups
+            vals = value_getter.call(row)
+            if vals.empty?
+              row[target] = nil
+            else
+              empty_replacer.process(row) if replace_empty
+              max_vals = find_max_vals(row)
+              add_constant(row, max_vals)
+              even_xform.process(row) if enforce_evenness
+              group_cleaner.process(row) if remove_empty_groups
+            end
+            row
           end
 
           private

diff --git a/spec/kiba/extend/transforms/reshape/fields_to_field_group_with_constant_spec.rb b/spec/kiba/extend/transforms/reshape/fields_to_field_group_with_constant_spec.rb
@@ -36,7 +36,7 @@
         {a_type: "a thing", a_note: "foo", a_date: "2022"},
         {a_type: "a thing", a_note: nil, a_date: "2022"},
         {a_type: "a thing", a_note: "foo", a_date: nil},
-        {a_type: nil, a_note: nil, a_date: nil},
+        {a_type: nil, a_note: "", a_date: nil},
         {a_type: "a thing|a thing|a thing", a_note: "foo|bar|baz",
          a_date: "2022|2021|%NULLVALUE%"},
         {a_type: "a thing|a thing", a_note: "foo|bar", a_date: nil},
@@ -52,6 +52,35 @@
     it "reshapes the columns as specified" do
       expect(result).to eq(expected)
     end
+
+    context "with only one fieldmap field" do
+      let(:input) do
+        [
+          {note: "foo"},
+          {note: nil},
+          {note: "bar|foo"}
+        ]
+      end
+      let(:params) do
+        {
+          fieldmap: {note: :a_note},
+          constant_target: :a_type,
+          constant_value: "a thing"
+        }
+      end
+
+      let(:expected) do
+        [
+          {a_type: "a thing", a_note: "foo"},
+          {a_type: nil, a_note: nil},
+          {a_type: "a thing|a thing", a_note: "bar|foo"}
+        ]
+      end
+
+      it "reshapes the columns as specified" do
+        expect(result).to eq(expected)
+      end
+    end
   end
 
   context "with `treat_as_null: %BLANK%`" do
@@ -69,7 +98,7 @@
         {a_type: "a thing", a_note: "foo", a_date: "2022"},
         {a_type: "a thing", a_note: nil, a_date: "2022"},
         {a_type: "a thing", a_note: "foo", a_date: nil},
-        {a_type: nil, a_note: nil, a_date: nil},
+        {a_type: nil, a_note: "", a_date: nil},
         {a_type: "a thing|a thing|a thing", a_note: "foo|bar|baz",
          a_date: "2022|2021|%BLANK%"},
         {a_type: "a thing|a thing", a_note: "foo|bar", a_date: nil},
@@ -102,7 +131,7 @@
         {a_type: "a thing", a_note: "foo", a_date: "2022"},
         {a_type: "a thing", a_note: nil, a_date: "2022"},
         {a_type: "a thing", a_note: "foo", a_date: nil},
-        {a_type: nil, a_note: nil, a_date: nil},
+        {a_type: nil, a_note: "", a_date: nil},
         {a_type: "a thing|a thing|a thing", a_note: "foo|bar|baz",
          a_date: "2022|2021|2021"},
         {a_type: "a thing|a thing", a_note: "foo|bar", a_date: nil},
@@ -135,7 +164,7 @@
         {a_type: "a thing", a_note: "foo", a_date: "2022"},
         {a_type: "a thing", a_note: nil, a_date: "2022"},
         {a_type: "a thing", a_note: "foo", a_date: nil},
-        {a_type: nil, a_note: nil, a_date: nil},
+        {a_type: nil, a_note: "", a_date: nil},
         {a_type: "a thing|a thing|a thing", a_note: "foo|bar|baz",
          a_date: "2022|2021"},
         {a_type: "a thing|a thing", a_note: "foo|bar", a_date: nil},
@@ -169,7 +198,7 @@
         {a_type: "a thing", a_note: "foo", a_date: "2022"},
         {a_type: "a thing", a_note: nil, a_date: "2022"},
         {a_type: "a thing", a_note: "foo", a_date: nil},
-        {a_type: nil, a_note: nil, a_date: nil},
+        {a_type: nil, a_note: "", a_date: nil},
         {a_type: "a thing|a thing|a thing", a_note: "foo|bar|baz",
          a_date: "2022|2021|%NULL%"},
         {a_type: "a thing|a thing", a_note: "foo|bar", a_date: nil},
@@ -210,7 +239,7 @@
       [
         {a_type: "a thing", a_note: "foo"},
         {a_type: nil, a_note: nil},
-        {a_type: nil, a_note: nil},
+        {a_type: nil, a_note: ""},
         {a_type: "a thing|a thing", a_note: "foo|bar"}
       ]
     end