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.

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

Add :sep as an alias for :col_sep #318

Closed
wants to merge 1 commit into from

Conversation

jsxs0
Copy link
Contributor

@jsxs0 jsxs0 commented Oct 28, 2024

This PR adds :sep as a shorter alias for :col_sep to improve usability:

  • Adds :sep option that acts as alias for :col_sep
  • Reduces keystrokes (3 vs 8) when specifying separator
  • Aligns with other data libraries like pandas (pd.read_csv(sep="\t"))
  • Maintains backward compatibility
  • Raises clear error if both options specified

Example usage:

# Old way
CSV.read("data.tsv", col_sep: "\t")

# New way
CSV.read("data.tsv", sep: "\t")

The change is motivated by:

  1. Making the interface more ergonomic with shorter option name
  2. Better alignment with common practices in other data libraries

Changes:

  1. Adds sep parameter to CSV#initialize
  2. Adds error check for both options
  3. Uses sep value if provided, otherwise falls back to col_sep

Inspired by #272

@kou
Copy link
Member

kou commented Oct 30, 2024

I'm negative this.
If we have a special API for TSV something like #319, we don't need to specify col_sep: explicitly in most use cases?

@kojix2
Copy link

kojix2 commented Nov 25, 2024

I agree with @jsxs0.
I still like the short appearance of sep and do not like the naming of col_sep.
However, I am not going to argue strongly about this. I am not a regular user of the CSV library.

@kou
Copy link
Member

kou commented Nov 26, 2024

OK.
Let's close this.

@kou kou closed this Nov 26, 2024
@kojix2
Copy link

kojix2 commented Nov 26, 2024

Hey, I would like to understand the reason why this PR is closed.

I imagine there are at least 100 people on the planet who would like this change using Ruby's CSV library. Or maybe more. These people may search for pull requests. I feel a kind of obligation to ask for an explanation.

Please understand that I am not asking for the issue to be reopened in order to introduce sep, but rather that I would like to hear your ideas and explanations behind the decision to close it.

Thank you.


I don't think that creating a new TSV class is a good enough reason to reject this pull request.
For example, there are many file formats that use “|” as a delimiter.

  • Compatibility
  • Keeping the code simple
  • Consistency with row_sep

There are various possibilities (or the possibility that none of these apply), but the reason is not clear from the outside.

@kou
Copy link
Member

kou commented Nov 27, 2024

Consistency with row_sep

This is the reason.

@kou
Copy link
Member

kou commented Nov 27, 2024

there are many file formats that use “|” as a delimiter.

It will be rare than TSV. Because there is no Pipe-Separated-Values name or something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants