chore: Remove pandas-like specific check_column_names_are_unique util#2749
Conversation
| len_unique_columns = len(set(columns)) | ||
| try: | ||
| len_unique_columns = len(set(columns)) | ||
| except TypeError as exc: # pragma: no cover |
There was a problem hiding this comment.
Set on unhashable type raises a TypeError:
set(([1], [2]))
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
Cell In[1], line 1
----> 1 set(([1], [2]))
TypeError: unhashable type: 'list'Should we raise a ValueError as before instead?
There was a problem hiding this comment.
Could you give an example of how we'd get columns like that in the places the function is currently used?
import pandas as pd
>>> pd.DataFrame([[1, 2, 3], [4, 5, 6]], columns=([1], [2], [3]))
TypeError: unhashable type: 'list'AFAIK,
pandasonly supportsHashablecolumn "names" - so I'm a little confused 🤔
We're checking an existing NativeFrame.columns - so I would have thought that condition is not reachable
There was a problem hiding this comment.
it is, there's a test which gets there
There was a problem hiding this comment.
it is, there's a test which gets there
Is there?
Why do we have a # pragma: no cover?
There was a problem hiding this comment.
If this is a pandas version-specific thing - then a comment explaining the no cover would help
I made the suggestion in (#2511 (comment)) based on the fact this appears unreachable
Follow-up to #2749 (comment) If CI can catch this, then I'll add a comment explaining where this is needed
chore: Do we really have coverage? Follow-up to #2749 (comment) If CI can catch this, then I'll add a comment explaining where this is needed
3 participants
What type of PR is this? (check all applicable)
Related issues
Spotted by @dangotbanned, see comment
Checklist