Skip to content

Support Ruby 3.2.x - small bug fixes #2

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.

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

linucs
Copy link

@linucs linucs commented Oct 13, 2023

No description provided.

@elohanlon
Copy link
Member

Hi @linucs. Thanks for opening this PR!

I had some recent in-progress changes that I merged today (and released as gem version 1.1.0), and I'm hoping that they'll cover your Ruby 3.2.x needs, and address the couple of casting bugs you found. Could you please check out the new version when you have a chance and let me know?

I also saw that you added converters: %i[numeric date] in your PR, in the CSV.foreach method call in JsonCsv::CsvToJson.csv_file_to_hierarchical_json_hash, but I think that could be a breaking change for current users of this gem, and also doesn't feel like a good fit here for a couple of reasons:

  1. This gem is meant for converting between CSV and JSON structures, and JSON doesn't have a Date type. It's completely fine for a user of this gem to convert date strings to Ruby Date objects after a CSV row has already been converted to a JSON hash, but I wouldn't want to do it as part of that CSV-to-JSON conversion step in the gem.
  2. I believe the numeric part can already be handled by a field_casting_rules parameter entry (but please let me know if I'm overlooking anything here).

@elohanlon
Copy link
Member

elohanlon commented Oct 24, 2023

  1. I believe the numeric part can already be handled by a field_casting_rules parameter entry (but please let me know if I'm overlooking anything here).

One other note about converters: There are certain scenarios where a CSV cell may hold a numeric string value, but a user doesn't necessarily want that numeric string value to be parsed as a number. I have some cases like that in another app that uses json_csv. That's where the field_casting_rules parameter comes in -- to support a user's preference on a field by field basis.

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.

2 participants