Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove map_column! and map_headers! #1590

Merged
merged 3 commits into from
Dec 7, 2021
Merged

Remove map_column! and map_headers! #1590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5cd000d
Remove map_column! and map_headers
aurelien-reeves Dec 7, 2021
4264dd9
Update CHANGELOG.md
aurelien-reeves Dec 7, 2021
67d0b7d
[skip-ci]Fix typo in CHANGELOG.md
aurelien-reeves Dec 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,13 @@ Please visit [cucumber/CONTRIBUTING.md](https://github.com/cucumber/cucumber/blo
([PR#1588](https://github.com/cucumber/cucumber-ruby/pull/1588)
[Issue#1581](https://github.com/cucumber/cucumber-ruby/issues/1581))

- Removed `DataTable#map_column!` and `DataTable#map_headers!`.
Those methods were error-prone and planned to be removed a long time ago. You
can use the immutable versions instead: `DataTable#map_column` and
`DataTable#map_headers`.
([PR#1590](https://github.com/cucumber/cucumber-ruby/pull/1590)
[Issue#1584](https://github.com/cucumber/cucumber-ruby/issues/1584))

### Security fixes

### Deprecated
Expand Down
2 changes: 1 addition & 1 deletion UPGRADING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Upgrading to 8.0.0

# The wire protocol
## The wire protocol

The built-in wire protocol has been removed.

Expand Down
49 changes: 14 additions & 35 deletions lib/cucumber/multiline_argument/data_table.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,9 @@ def eof; end
NULL_CONVERSIONS = Hash.new(strict: false, proc: ->(cell_value) { cell_value }).freeze

# @param data [Core::Test::DataTable] the data for the table
# @param conversion_procs [Hash] see map_columns!
# @param header_mappings [Hash] see map_headers!
# @param header_conversion_proc [Proc] see map_headers!
# @param conversion_procs [Hash] see map_column
# @param header_mappings [Hash] see map_headers
# @param header_conversion_proc [Proc] see map_headers
def initialize(data, conversion_procs = NULL_CONVERSIONS.dup, header_mappings = {}, header_conversion_proc = nil)
raise ArgumentError, 'data must be a Core::Test::DataTable' unless data.is_a? Core::Test::DataTable

Expand Down Expand Up @@ -108,13 +108,6 @@ def location
@ast_table.location
end

# Creates a copy of this table, inheriting any column and header mappings
# registered with #map_column! and #map_headers!.
#
def dup
self.class.new(Core::Test::DataTable.new(raw), @conversion_procs.dup, @header_mappings.dup, @header_conversion_proc)
end

# Returns a new, transposed table. Example:
#
# | a | 7 | 4 |
Expand Down Expand Up @@ -142,7 +135,7 @@ def transpose
#
# [{'a' => '2', 'b' => '3', 'sum' => '5'}, {'a' => '7', 'b' => '9', 'sum' => '16'}]
#
# Use #map_column! to specify how values in a column are converted.
# Use #map_column to specify how values in a column are converted.
#
def hashes
@hashes ||= build_hashes
Expand Down Expand Up @@ -230,7 +223,8 @@ def match(pattern)
pattern.match(header_to_match)
end

# Redefines the table headers. This makes it possible to use
# Returns a new Table where the headers are redefined.
# This makes it possible to use
# prettier and more flexible header names in the features. The
# keys of +mappings+ are Strings or regular expressions
# (anything that responds to #=== will work) that may match
Expand All @@ -246,56 +240,41 @@ def match(pattern)
# A StepDefinition receiving this table can then map the columns
# with both Regexp and String:
#
# table.map_headers!(/phone( number)?/i => :phone, 'Address' => :address)
# table.map_headers(/phone( number)?/i => :phone, 'Address' => :address)
# table.hashes
# # => [{:phone => '123456', :address => 'xyz'}, {:phone => '345678', :address => 'abc'}]
#
# You may also pass in a block if you wish to convert all of the headers:
#
# table.map_headers! { |header| header.downcase }
# table.map_headers { |header| header.downcase }
# table.hashes.keys
# # => ['phone number', 'address']
#
# When a block is passed in along with a hash then the mappings in the hash take precendence:
#
# table.map_headers!('Address' => 'ADDRESS') { |header| header.downcase }
# table.map_headers('Address' => 'ADDRESS') { |header| header.downcase }
# table.hashes.keys
# # => ['phone number', 'ADDRESS']
#
def map_headers!(mappings = {}, &block)
# TODO: Remove this method for 2.0
clear_cache!
@header_mappings = mappings
@header_conversion_proc = block
end

# Returns a new Table where the headers are redefined. See #map_headers!
def map_headers(mappings = {}, &block)
self.class.new(Core::Test::DataTable.new(raw), @conversion_procs.dup, mappings, block)
end

# Returns a new Table with an additional column mapping.
#
# Change how #hashes converts column values. The +column_name+ argument identifies the column
# and +conversion_proc+ performs the conversion for each cell in that column. If +strict+ is
# true, an error will be raised if the column named +column_name+ is not found. If +strict+
# is false, no error will be raised. Example:
#
# Given /^an expense report for (.*) with the following posts:$/ do |table|
# posts_table.map_column!('amount') { |a| a.to_i }
# posts_table = posts_table.map_column('amount') { |a| a.to_i }
# posts_table.hashes.each do |post|
# # post['amount'] is a Fixnum, rather than a String
# end
# end
#
# rubocop:disable Style/OptionalBooleanParameter # the optional boolean parameter is kept for retrocompatibility
def map_column!(column_name, strict = true, &conversion_proc)
# TODO: Remove this method for 2.0
@conversion_procs[column_name.to_s] = { strict: strict, proc: conversion_proc }
self
end
# rubocop:enable Style/OptionalBooleanParameter

# Returns a new Table with an additional column mapping. See #map_column!
# rubocop:disable Style/OptionalBooleanParameter # the optional boolean parameter is kept for retrocompatibility
def map_column(column_name, strict = true, &conversion_proc)
aurelien-reeves marked this conversation as resolved.
Show resolved Hide resolved
conversion_procs = @conversion_procs.dup
conversion_procs[column_name.to_s] = { strict: strict, proc: conversion_proc }
Expand All @@ -318,8 +297,8 @@ def map_column(column_name, strict = true, &conversion_proc)
# where the difference actually is.
#
# Since all tables that are passed to StepDefinitions always have String
# objects in their cells, you may want to use #map_column! before calling
# #diff!. You can use #map_column! on either of the tables.
# objects in their cells, you may want to use #map_column before calling
# #diff!. You can use #map_column on either of the tables.
#
# A Different error is raised if there are missing rows or columns, or
# surplus rows. An error is <em>not</em> raised for surplus columns. An
Expand Down
101 changes: 8 additions & 93 deletions spec/cucumber/multiline_argument/data_table_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,52 +58,6 @@ module MultilineArgument
end
end

describe '#map_column!' do
it 'should allow mapping columns' do
@table.map_column!('one', &:to_i)
expect(@table.hashes.first['one']).to eq 4444
end

it 'applies the block once to each value' do
headers = ['header']
rows = ['value']
table = DataTable.from [headers, rows]
count = 0
table.map_column!('header') { |_value| count += 1 }
table.rows
expect(count).to eq rows.size
end

it 'should allow mapping columns and take a symbol as the column name' do
@table.map_column!(:one, &:to_i)
expect(@table.hashes.first['one']).to eq 4444
end

it 'should allow mapping columns and modify the rows as well' do
@table.map_column!(:one, &:to_i)
expect(@table.rows.first).to include(4444)
expect(@table.rows.first).to_not include('4444')
end

it 'should pass silently if a mapped column does not exist in non-strict mode' do
expect do
@table.map_column!('two', false, &:to_i)
@table.hashes
end.not_to raise_error
end

it 'should fail if a mapped column does not exist in strict mode' do
expect do
@table.map_column!('two', true, &:to_i)
@table.hashes
end.to raise_error('The column named "two" does not exist')
end

it 'should return the table' do
expect(@table.map_column!(:one, &:to_i)).to eq @table
end
end

describe '#map_column' do
it 'should allow mapping columns' do
new_table = @table.map_column('one', &:to_i)
Expand Down Expand Up @@ -212,42 +166,6 @@ module MultilineArgument
end
end

describe '#map_headers!' do
let(:table) do
DataTable.from([
%w[HELLO WORLD],
%w[4444 55555]
])
end

it 'renames the columns to the specified values in the provided hash' do
@table.map_headers!('one' => :three)
expect(@table.hashes.first[:three]).to eq '4444'
end

it 'allows renaming columns using regexp' do
@table.map_headers!(/one|uno/ => :three)
expect(@table.hashes.first[:three]).to eq '4444'
end

it 'copies column mappings' do
@table.map_column!('one', &:to_i)
@table.map_headers!('one' => 'three')
expect(@table.hashes.first['three']).to eq 4444
end

it 'takes a block and operates on all the headers with it' do
table.map_headers!(&:downcase)
expect(table.hashes.first.keys).to match %w[hello world]
end

it 'treats the mappings in the provided hash as overrides when used with a block' do
table.map_headers!('WORLD' => 'foo', &:downcase)

expect(table.hashes.first.keys).to match %w[hello foo]
end
end

describe '#map_headers' do
let(:table) do
DataTable.from([
Expand All @@ -267,8 +185,8 @@ module MultilineArgument
end

it 'copies column mappings' do
@table.map_column!('one', &:to_i)
table2 = @table.map_headers('one' => 'three')
table = @table.map_column('one', &:to_i)
table2 = table.map_headers('one' => 'three')
expect(table2.hashes.first['three']).to eq 4444
end

Expand Down Expand Up @@ -467,12 +385,11 @@ module MultilineArgument
%w[name male],
%w[aslak true]
])
t1.map_column!('male') { |m| m == 'true' }
t2 = DataTable.from([
%w[name male],
['aslak', true]
])
t1.diff!(t2)
t1.map_column('male') { |m| m == 'true' }.diff!(t2)
expect(t1.to_s(indent: 12, color: false)).to eq %(
| name | male |
| aslak | true |
Expand All @@ -484,14 +401,11 @@ module MultilineArgument
%w[name male],
['aslak', true]
])
t1.map_column!('male') do
'true'
end
t2 = DataTable.from([
%w[name male],
%w[aslak true]
])
t2.diff!(t1)
t2.diff!(t1.map_column('male') { 'true' })
expect(t1.to_s(indent: 12, color: false)).to eq %(
| name | male |
| aslak | true |
Expand All @@ -503,8 +417,9 @@ module MultilineArgument
%w[Name Male],
%w[aslak true]
])
t1.map_headers!('Name' => 'name', 'Male' => 'male')
t1.map_column!('male') { |m| m == 'true' }
t1 = t1.map_headers('Name' => 'name', 'Male' => 'male')
t1 = t1.map_column('male') { |m| m == 'true' }

t2 = DataTable.from([
%w[name male],
['aslak', true]
Expand Down Expand Up @@ -540,7 +455,7 @@ module MultilineArgument
%w[Foo Bar]
])
expect do
t1.map_headers!(/uk/ => 'u')
t1 = t1.map_headers(/uk/ => 'u')
t1.hashes
end.to raise_error(%(2 headers matched /uk/: ["Cuke", "Duke"]))
end
Expand Down