Bug Fix: #60343 Construction of Series / Index fails from dict keys when "str" dtype is specified explicitly #60383
Conversation
|
Hi, I'm a student contributing to this PR and am on bit of a time crunch due to finals. For my school project, my task is to merge the PR as quickly as possible with the help and guidance of maintainers. I was able to fix the bug but I am a bit stuck on how to fix the checks. Could @jorisvandenbossche or anyone else help? Especially the unit tests ones. I tried to fix the pre-commit (using ruff lint fix) but every time I fixed a formatting issue, after running pre-commit it goes to the initial position before I did the fix. For the Doc build and upload check (it was giving an error for every declaration of ipython that didn't have import pandas as pd), I manually inserted it but don't know if there is an easy way. |
jorisvandenbossche
added
Strings
That should normally not have been needed. Did you get those errors locally? (in that case maybe something with the set up was wrong) |
| return ( | ||
| isinstance(arr_or_dtype, np.dtype) | ||
| and arr_or_dtype == "object" | ||
| or isinstance(arr_or_dtype, StringDtype) |
There was a problem hiding this comment.
We don't want to change the meaning of is_object_dtype to also include StringDtype. What was the reason you needed this change?
| df = DataFrame( | ||
| { | ||
| "A": range(3), | ||
| "B": range(3), | ||
| "C": range(3) | ||
| } | ||
| ).rename(columns={"B": "A"}) | ||
| df = DataFrame({"A": range(3), "B": range(3), "C": range(3)}).rename( | ||
| columns={"B": "A"} | ||
| ) |
There was a problem hiding this comment.
It seems you included some unintended formatting changes. Maybe some setting in the IDE you are using that is conflicting with the formatting defaults in pandas?
I would recommend you to set up the pre-commit hook, which will ensure the code is formatted correctly when committing (see https://pandas.pydata.org/docs/development/contributing_codebase.html#pre-commit)
| if isinstance(data, KeysView): | ||
| data = list(data) |
There was a problem hiding this comment.
I think this is the critical part that is indeed fixing the issue (for Series(..) at least), and so this is a good change.
I would just make the if check more generic. Because while the example was using dict.keys(), you also have other iterables (e.g. the dict.values()) that will have the same problem, and we should try to fix it for all of them.
Looking at the logic just below for the non-ExtensionDtype cases, there is a if hasattr(data, "__array__") and then after that in the final else it is also doing a data = list(data). So maybe the check above could be if not hasattr(data, "__array__") instead of if isinstance(data, KeysView).
The default behaviour (pd.Index(d.keys())) worked correctly, but explicitly setting dtype="str" raised a ValueError. The issue stemmed from dict_keys not being converted to a proper array-like structure before being passed to StringDtype, which couldn't handle such inputs.
To fix the issue:
KeyView was introduced to identify and preprocess dict_keys before passing them to Pandas internals. The keys are now converted to a list for compatibility.
Updated logic in Index and sanitize_array to map dtype="str" to StringDtype(storage="python"). Updated check_array_indexer to allow empty boolean indexers for StringArray
New test added "test_index_from_dict_keys_with_dtype" to ensure:
Default inference (pd.Index(d.keys())) works.
Explicit dtype="str" works, resulting in string[python].
Updated existing tests (test_is_object and test_empty_fancy) to handle new behaviours introduced by the fix.
After the fix both the default (pd.Index(d.keys())) and explicit (pd.Index(d.keys(), dtype="str")) cases work:
