-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
ENH: add Series.info #31796
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
ENH: add Series.info #31796
Conversation
cfa80b3
to
362a224
Compare
Hello @MarcoGorelli! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-09-19 16:26:44 UTC |
Thanks @WillAyd for your review, have updated |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so, before doing this, can you do a pre-cursor which moves the .info code into pandas/io/formats/info.py. we already have way too many things inside the core/frame.py and core/generic.py modules. I think this would promote easy sharing of this code. also happy for the added tests, you can move them as well to pandas/io/formats/test_info.py
4d88b07
to
abbae9a
Compare
pandas/io/formats/info.py
Outdated
cols = Index([data.name]) | ||
dtypes = Index([data.dtypes]) | ||
|
||
col_count = len(cols) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I think this is confusing to have col_count
be populated by a Series in some cases - can you think of a way to refactor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WillAyd sure, have given this a go
I've pushed a new attempt at this. I've factored out some pieces of |
@MarcoGorelli can you fix up the PR here? Someone will take a look again thereafter |
@WillAyd Sure, have given another go at it, now using a namedtuple to group together some similar variables |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is headed in the right direction
@@ -15,6 +15,32 @@ | |||
from pandas.core.series import Series # noqa: F401 | |||
|
|||
|
|||
class CountConfigs(NamedTuple): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you generate this using collections.namedtuple
instead? We typically don't subclass things from typing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, can do, although I think this is the newer syntax, it's taken directly from https://docs.python.org/3/library/typing.html#typing.NamedTuple
They also recommend it in the mypy docs: https://mypy.readthedocs.io/en/stable/kinds_of_types.html#named-tuples
(closing as @ivanovmg will take it over) |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff
screenshot of the example

Note to self: this change from
ba72b59b44f15076877e784e16cbc66aa03e0adc
breaks the testsUpdated
Currently, looks like this: