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

isinstance check against mutable mappings rather than dicts #6792

Closed
hayd opened this issue Apr 4, 2014 · 7 comments
Closed

isinstance check against mutable mappings rather than dicts #6792

hayd opened this issue Apr 4, 2014 · 7 comments
Labels
Enhancement Internals Related to non-user accessible pandas implementation

Comments

@hayd
Copy link
Contributor

hayd commented Apr 4, 2014

atm we're checking dict-like by doing isinstance(x, dict), I think we should be using collections.MutableMapping, slightly more general.

@jreback jreback added this to the 0.15.0 milestone Apr 4, 2014
@jreback
Copy link
Contributor

jreback commented Apr 4, 2014

yep

@jtratner
Copy link
Contributor

it really is noticeably slower to use MutableMapping instead of dict (and unfortunately you incur the cost for every comparison). The perf difference isn't huge, but I've found it's enough to cause a problem in tight inner loops (e.g., wouldn't want to do this in type inference code from apply). mostly an FYI

@nicktimko
Copy link

Using the more general collections.Mapping would be preferable in the basic read cases (e.g. making a DataFrame). I have an lazy object that inherits from collections.abc.Mapping and I can't use it to initialize a DF, but if I pd.DataFrame(dict(mymapping)) it works just fine. Bit strange.

@jreback
Copy link
Contributor

jreback commented Aug 12, 2014

I think this is possible. You are welcome to submit a pull-request, with tests for this functionaility

@cpcloud
Copy link
Member

cpcloud commented Aug 12, 2014

Are there a lot of mappings that are not dicts? Having to call dict is kind of a minor inconvenience compared to a performance hit for which there is no user facing solution. On the flip side I can see why this is useful since maybe you don't want to be coupled to dicts and you just want to implement the interface. Need performance tests as well.

@immerrr
Copy link
Contributor

immerrr commented Dec 13, 2014

I have always thought that isinstance(x, CLS) (or rather issubclass(type(x), CLS)) lends itself to caching pretty well, given that user types don't change that frequently.

I was in fact going to suggest to use singledispatch to improve structure of multi-branch ifs that implement overloading on parameter types and I'm pretty sure it caches the type-to-function-overload lookup somewhere under the hood, so if not borrowing actual code we could borrow some inspiration from it.

@jreback jreback modified the milestones: 0.16.0, Next Major Release Mar 6, 2015
@mroeschke mroeschke added Internals Related to non-user accessible pandas implementation and removed API Design labels Apr 11, 2021
@mroeschke mroeschke removed this from the Contributions Welcome milestone Oct 13, 2022
@mroeschke
Copy link
Member

Closing in favor of #58803

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Internals Related to non-user accessible pandas implementation
Projects
None yet
Development

No branches or pull requests

7 participants