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

DataFrame.to_dict(orient="index") should return a nested dictionary #799

Open
gandhis1 opened this issue Oct 18, 2023 · 2 comments
Open

Comments

@gandhis1
Copy link
Contributor

Describe the bug
When you do DataFrame.to_dict(orient="index"), and you are using default integer index and string columns, you will get a Dict[int, Dict[str, Any]] at runtime. The current annotations do not reflect this nested dictionary, they instead return a Dict[Hashable, Any]. I understand that the exact type this returns has changed slightly after recent commits by @twoertwein, but the issue still stands.

Expectation: returns MutableMapping[Hashable, MutableMapping[Hashable, Any]]

This issue could be broadened to more precisely annotate all possible values of orient if desired. For instance, orient="split" probably should return a TypedDict that has index and columns as keys.

To Reproduce
Any usage of to_dict(orient="index") should reproduce this. Below is a screenshot of an example:

image

@twoertwein
Copy link
Member

The current annotations do not reflect this nested dictionary, they instead return a Dict[Hashable, Any]. I understand that the exact type this returns has changed slightly after recent commits by @twoertwein, but the issue still stands.

Expectation: returns MutableMapping[Hashable, MutableMapping[Hashable, Any]]

MutableMapping[Hashable, Any] (the current return annotation) should be compatible with MutableMapping[Hashable, MutableMapping[Hashable, Any]] except that you don't get type checking for the Any part.

DataFrame is not generic in the index and columns, so the best we could do is what you suggested MutableMapping[Hashable, MutableMapping[Hashable, Any]]. Feel free to open a PR (including tests) but let's first wait for what @Dr-Irv thinks about that.

@Dr-Irv
Copy link
Collaborator

Dr-Irv commented Oct 18, 2023

DataFrame is not generic in the index and columns, so the best we could do is what you suggested MutableMapping[Hashable, MutableMapping[Hashable, Any]]. Feel free to open a PR (including tests) but let's first wait for what @Dr-Irv thinks about that.

I'm fine with expanding the overloads on to_dict() to handle the different cases of the orient value. There will be a bunch of new overloads, but we can also be incremental about it.

Agree that the best we can do is MutableMapping[Hashable, MutableMapping[Hashable, Any]], but also have to take into account the value of the into argument.

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

No branches or pull requests

3 participants