From fe6944e7d8a9a9e70f0230b1f60d7f671202d165 Mon Sep 17 00:00:00 2001 From: Kristina Spurgin Date: Mon, 20 Nov 2023 19:16:59 -0500 Subject: [PATCH] Fix bug in Reshape::FieldsToFieldGroupWithConstant This was triggered only when the length of the given fieldmap was 1 --- CHANGELOG.adoc | 1 + .../fields_to_field_group_with_constant.rb | 56 +++++++++++++------ ...ields_to_field_group_with_constant_spec.rb | 41 ++++++++++++-- 3 files changed, 76 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.adoc b/CHANGELOG.adoc index 7fd3ec55e..d55a4dd43 100644 --- 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 index 7957f3918..234c3a840 100644 --- 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 index e300410ee..70dffd47d 100644 --- 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