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

Name to lowercase in renameDuplicateColumnHeaders #1197

Merged
merged 2 commits into from
Apr 19, 2023

Conversation

s1ny1998
Copy link
Contributor

@s1ny1998 s1ny1998 commented Mar 27, 2023

renameDuplicateColumnHeaders when allowDuplicateColumnNames is true is case sensitive which causes validateColumn(final Column<?> newColumn) to fail and throw IllegalArgumentException.

What was changed

Used toLowerCase while using HashMap.
Also thought of using treeMap, but get() in treeMap has timecomplexity of O(log(n))

Did you add a unit test?
No

@s1ny1998 s1ny1998 changed the title name to lowercase in renameDuplicateColumnHeaders Name to lowercase in renameDuplicateColumnHeaders Mar 27, 2023
@benmccann benmccann merged commit a6d644d into jtablesaw:master Apr 19, 2023
ccleva added a commit to tlabs-data/tablesaw that referenced this pull request Dec 15, 2024
ccleva pushed a commit to tlabs-data/tablesaw that referenced this pull request Dec 15, 2024
ccleva added a commit to tlabs-data/tablesaw that referenced this pull request Dec 15, 2024
ccleva pushed a commit to tlabs-data/tablesaw that referenced this pull request Dec 15, 2024
@ccleva
Copy link
Contributor

ccleva commented Dec 16, 2024

Hi @s1ny1998. A fix based on your code is available in the new maintenance release from the maintenance fork. See this discussion for details.

ccleva added a commit to tlabs-data/tablesaw that referenced this pull request Jan 3, 2025
@ccleva ccleva mentioned this pull request Jan 3, 2025
1 task
benmccann pushed a commit that referenced this pull request Jan 3, 2025
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