Skip to content

BUG: Exception when frame constructed from dict of iterators #26356

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
May 20, 2019
Merged

BUG: Exception when frame constructed from dict of iterators #26356

merged 3 commits into from
May 20, 2019

Conversation

topper-123
Copy link
Contributor

Allows iterators as values when instantiating DataFrames using dicts.

Also collected the iterable-to-list functionality from core/series.py into core/common.py, so it is shared.

jreback
jreback reviewed May 12, 2019
View reviewed changes
pandas/core/common.py Outdated
@@ -289,6 +289,15 @@ def maybe_make_list(obj):
return obj


def maybe_itarable_to_list(obj: Any) -> Any:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this routine already exists, look around in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did look and have looked again now. It is not there. There's a _iterable_not_string but that's different.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't the signature be Union[Iterable, Any] -> Union[List, Any]

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think they're equivalent,as Union[Iterable, Any] is the same as Any? E.g. if the returned value from maybe_itarable_to_list is called x, if could cause an error to call x.append, if x was e.g. an int.

Both version pass the mypi checks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right but the 2nd is much more informative to a reader

@codecov
Copy link

codecov bot commented May 12, 2019

Codecov Report

Merging #26356 into master will decrease coverage by <.01%.
The diff coverage is 87.5%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26356      +/-   ##
==========================================
- Coverage   92.04%   92.03%   -0.01%     
==========================================
  Files         175      175              
  Lines       52289    52293       +4     
==========================================
  Hits        48130    48130              
- Misses       4159     4163       +4
Flag Coverage Δ
#multiple 90.59% <87.5%> (ø) ⬆️
#single 40.71% <75%> (-0.15%) ⬇️
Impacted Files Coverage Δ
pandas/core/common.py 98.42% <100%> (+0.03%) ⬆️
pandas/core/internals/construction.py 95.89% <100%> (+0.01%) ⬆️
pandas/core/series.py 93.66% <66.66%> (-0.01%) ⬇️
pandas/io/gbq.py 78.94% <0%> (-10.53%) ⬇️
pandas/core/frame.py 97.01% <0%> (-0.12%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e4f5bd...4986677. Read the comment docs.

@codecov
Copy link

codecov bot commented May 12, 2019 •
edited
Loading

Codecov Report

Merging #26356 into master will decrease coverage by <.01%.
The diff coverage is 90.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26356      +/-   ##
==========================================
- Coverage   91.74%   91.73%   -0.01%     
==========================================
  Files         174      174              
  Lines       50748    50752       +4     
==========================================
  Hits        46557    46557              
- Misses       4191     4195       +4
Flag Coverage Δ
#multiple 90.24% <90.9%> (ø) ⬆️
#single 41.7% <81.81%> (-0.09%) ⬇️
Impacted Files Coverage Δ
pandas/core/common.py 98.42% <100%> (+0.03%) ⬆️
pandas/core/internals/construction.py 95.93% <100%> (+0.01%) ⬆️
pandas/core/series.py 93.66% <75%> (-0.01%) ⬇️
pandas/io/gbq.py 78.94% <0%> (-10.53%) ⬇️
pandas/core/frame.py 97.02% <0%> (-0.12%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f44ac06...29834d7. Read the comment docs.

@topper-123 topper-123 force-pushed the frame_construct_from_dict_iterators branch 2 times, most recently from 28c2b86 to ee6d6be Compare May 12, 2019 17:48
@gfyoung gfyoung added Bug Series Series data structure labels May 12, 2019
@topper-123 topper-123 force-pushed the frame_construct_from_dict_iterators branch from ee6d6be to 8b986d6 Compare May 12, 2019 20:05
@topper-123 topper-123 changed the title BUG: Exception when frame constructed from dict of iterators WIP: BUG: Exception when frame constructed from dict of iterators May 12, 2019
@topper-123 topper-123 changed the title WIP: BUG: Exception when frame constructed from dict of iterators WIP/BUG: Exception when frame constructed from dict of iterators May 12, 2019
@topper-123
Copy link
Contributor Author

Trying something, not the final PR.

@topper-123 topper-123 force-pushed the frame_construct_from_dict_iterators branch 2 times, most recently from 52a1ff8 to 5717015 Compare May 13, 2019 20:19
@topper-123 topper-123 changed the title WIP/BUG: Exception when frame constructed from dict of iterators BUG: Exception when frame constructed from dict of iterators May 13, 2019
@topper-123 topper-123 force-pushed the frame_construct_from_dict_iterators branch 2 times, most recently from ee63d3e to ce6128a Compare May 13, 2019 21:48
@topper-123
Copy link
Contributor Author

I figured out the problem.

@topper-123
Copy link
Contributor Author

Is this ok?

jreback
jreback requested changes May 18, 2019
View reviewed changes
pandas/core/common.py Outdated
@@ -289,6 +289,15 @@ def maybe_make_list(obj):
return obj


def maybe_itarable_to_list(obj: Any) -> Any:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't the signature be Union[Iterable, Any] -> Union[List, Any]

?

@jreback jreback added this to the 0.25.0 milestone May 18, 2019
@topper-123 topper-123 force-pushed the frame_construct_from_dict_iterators branch 2 times, most recently from 664f489 to c63f418 Compare May 18, 2019 20:22
@jreback
Copy link
Contributor

jreback commented May 19, 2019

lgtm. if you can update the annotation, ping on green.

@topper-123 topper-123 force-pushed the frame_construct_from_dict_iterators branch from c63f418 to 29834d7 Compare May 19, 2019 20:06
@jreback jreback merged commit 44d5498 into pandas-dev:master May 20, 2019
@jreback
Copy link
Contributor

jreback commented May 20, 2019

thanks @topper-123 keep em coming!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonjayhawkins simonjayhawkins simonjayhawkins left review comments

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Bug Series Series data structure
Projects
None yet
Milestone
0.25.0
Development

Successfully merging this pull request may close these issues.

BUG: Instantiating DataFrames using dicts with iterators/generators as values causes errors
4 participants
@topper-123 @jreback @simonjayhawkins @gfyoung