Skip to content

Support block in CSV::Row#to_h#356

Open
tsymbalenkovlad wants to merge 8 commits intoruby:mainfrom
tsymbalenkovlad:main
Open

Support block in CSV::Row#to_h#356
tsymbalenkovlad wants to merge 8 commits intoruby:mainfrom
tsymbalenkovlad:main

Conversation

@tsymbalenkovlad
Copy link
Copy Markdown

It will be consistent with Ruby's method, otherwise it has to be like row.entries.to_h { ... }

Comment thread lib/csv/row.rb Outdated
new_key, new_value = if block_given?
yield(key, value)
else
[key, value]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't want to create a needless temporary Array.

Could you use

if block_given?
  each do |key, value|
    key, value = yield(key, value)
    # ...
  end
else
  each do |key, value|
    # ...
  end
end

or

each do |key, value|
  key, value = yield(key, value) if block_given?
  # ...
end

?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, that's a good one

Comment thread test/csv/test_row.rb Outdated
Comment on lines +342 to +344
row2 = CSV::Row.new(%w{A B C}, [1, 2, 3])
hash2 = row2.to_hash { |k, v| [k, v ** 2] }
assert_equal({"A" => 1, "B" => 4, "C" => 9}, hash2)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you create a separated test?

@tsymbalenkovlad tsymbalenkovlad requested a review from kou April 30, 2026 09:37
Comment thread lib/csv/row.rb Outdated
Comment on lines +655 to +656
each do |key, value|
key, value = yield(key, value) if block_given?
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry. We need to call self[key]:

Suggested change
each do |key, value|
key, value = yield(key, value) if block_given?
each do |key, _value|
value = self[key]
key, value = yield(key, value) if block_given?

@tsymbalenkovlad tsymbalenkovlad requested a review from kou April 30, 2026 10:11
Comment thread test/csv/test_row.rb Outdated
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds support for passing a block to CSV::Row#to_h / #to_hash, aligning usage with Ruby’s to_h { ... } pattern and avoiding the need for row.entries.to_h { ... }.

Changes:

  • Add a new test covering CSV::Row#to_hash with a block.
  • Update CSV::Row#to_h to yield each key/value pair to an optional block before insertion.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.

File Description
test/csv/test_row.rb Adds a test for block form of to_hash.
lib/csv/row.rb Implements yielding to an optional block in to_h during hash construction.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/csv/row.rb Outdated
Comment on lines +655 to +659
each do |key, _value|
hash[key] = self[key] unless hash.key?(key)
value = self[key]
key, value = yield(key, value) if block_given?

hash[key] = value unless hash.key?(key)
Comment thread lib/csv/row.rb
Comment on lines 655 to +657
each do |key, _value|
hash[key] = self[key] unless hash.key?(key)
value = self[key]
key, value = yield(key, value) if block_given?
Comment thread lib/csv/row.rb Outdated
Comment on lines +653 to +659
def to_h
hash = {}
each do |key, _value|
hash[key] = self[key] unless hash.key?(key)
value = self[key]
key, value = yield(key, value) if block_given?

hash[key] = value unless hash.key?(key)
Comment thread test/csv/test_row.rb Outdated
Comment on lines +345 to +346
row = CSV::Row.new(%w{A B C}, [1, 2, 3])
hash = row.to_hash { |k, v| [k, v ** 2] }
Comment thread lib/csv/row.rb
Comment on lines 653 to +657
def to_h
hash = {}
each do |key, _value|
hash[key] = self[key] unless hash.key?(key)
value = self[key]
key, value = yield(key, value) if block_given?
@tsymbalenkovlad tsymbalenkovlad requested a review from kou April 30, 2026 12:01
Comment thread lib/csv/row.rb
hash = {}
each do |key, _value|
hash[key] = self[key] unless hash.key?(key)
next if hash.key?(key)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

key may be changed by yield(key, value). So we can't do this if block_given?.

How about using separated each?

if block_given?
  each do |key, _value|
    key, value = yield(key, self[key])
    hash[key] = value unless hash.key?(key)
  end
else
  each do |key, _value|
    hash[key] = self[key] unless hash.key?(key)
  end
end

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds block support to CSV::Row#to_h (and therefore #to_hash) to align with Ruby’s to_h { ... } conversion pattern, so callers don’t need to go through row.entries.to_h { ... }.

Changes:

  • Extend CSV::Row#to_h to accept an optional block that transforms each (key, value) pair before insertion.
  • Document the new block form in CSV::Row#to_h’s RDoc.
  • Add a unit test exercising to_hash with a block.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
test/csv/test_row.rb Adds coverage for CSV::Row#to_hash accepting a block.
lib/csv/row.rb Implements and documents block support for CSV::Row#to_h/#to_hash.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread lib/csv/row.rb
Comment on lines 662 to +668
each do |key, _value|
hash[key] = self[key] unless hash.key?(key)
next if hash.key?(key)

value = self[key]
key, value = yield(key, value) if block_given?

hash[key] = value
Comment thread lib/csv/row.rb
next if hash.key?(key)

value = self[key]
key, value = yield(key, value) if block_given?
Comment thread test/csv/test_row.rb
Comment on lines +345 to +347
row = CSV::Row.new(%w{A A B C}, [1, 2, 2, 3])
hash = row.to_hash { |k, v| [k, v**2] }
assert_equal({"A" => 1, "B" => 4, "C" => 9}, hash)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants