Skip to content

DOC: Fixed errors in doc string for Categorical + cleanup #17655

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 1 commit into from
Sep 25, 2017
Merged

DOC: Fixed errors in doc string for Categorical + cleanup #17655

merged 1 commit into from
Sep 25, 2017

Conversation

topper-123
Copy link
Contributor

The doc string for Categorical has the examples wrong wrt. orderedness. This fixes that + some cleanup.

jorisvandenbossche
jorisvandenbossche reviewed Sep 24, 2017
View reviewed changes
Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

Thanks! Some minor comments

pandas/core/categorical.py Outdated

>>> Categorical(['a', 'b', 'c', 'a', 'b', 'c'])
[a, b, c, a, b, c]
Categories (3, object): [a < b < c]
Categories (3, object): [a, b, c]

Only ordered `Categoricals` can be sorted (according to the order
Copy link
Member

Choose a reason for hiding this comment

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

This is actually incorrect: unordered categoricals can be sorted, but it is true they do not have a min and max

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've changed the wording of the paragraph.

pandas/core/categorical.py Outdated
@@ -199,18 +199,21 @@ class Categorical(PandasObject):
>>> from pandas import Categorical
Copy link
Member

Choose a reason for hiding this comment

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

while you are at it, can you remove this line and update the others to use pd.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok

@jorisvandenbossche jorisvandenbossche added this to the 0.21.0 milestone Sep 24, 2017
@pep8speaks
Copy link

pep8speaks commented Sep 24, 2017 •
edited
Loading

Hello @topper-123! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on September 24, 2017 at 14:14 Hours UTC

@codecov
Copy link

codecov bot commented Sep 24, 2017

Codecov Report

Merging #17655 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17655      +/-   ##
==========================================
- Coverage   91.26%   91.24%   -0.02%     
==========================================
  Files         163      163              
  Lines       49776    49776              
==========================================
- Hits        45426    45417       -9     
- Misses       4350     4359       +9
Flag Coverage Δ
#multiple 89.03% <ø> (ø) ⬆️
#single 40.28% <ø> (-0.07%) ⬇️
Impacted Files Coverage Δ
pandas/core/categorical.py 95.27% <ø> (ø) ⬆️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.77% <0%> (-0.1%) ⬇️

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 c95eb38...860ce75. Read the comment docs.

@codecov
Copy link

codecov bot commented Sep 24, 2017 •
edited
Loading

Codecov Report

Merging #17655 into master will decrease coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17655      +/-   ##
==========================================
- Coverage   91.26%   91.24%   -0.03%     
==========================================
  Files         163      163              
  Lines       49806    49806              
==========================================
- Hits        45455    45445      -10     
- Misses       4351     4361      +10
Flag Coverage Δ
#multiple 89.04% <ø> (-0.01%) ⬇️
#single 40.3% <ø> (-0.07%) ⬇️
Impacted Files Coverage Δ
pandas/core/categorical.py 95.28% <ø> (ø) ⬆️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/frame.py 97.73% <0%> (-0.1%) ⬇️
pandas/core/indexes/datetimes.py 95.44% <0%> (-0.1%) ⬇️

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 965c1c8...b342678. Read the comment docs.

@topper-123 topper-123 force-pushed the categorical_docs branch 2 times, most recently from 4558ca5 to a0e4489 Compare September 24, 2017 14:06
@topper-123
Copy link
Contributor Author

I've changed the url under Notes to a full url, as per @jreback 's comment in this #17653.

@jorisvandenbossche jorisvandenbossche merged commit 0d06216 into pandas-dev:master Sep 25, 2017
@jorisvandenbossche
Copy link
Member

Thanks!

@topper-123 topper-123 mentioned this pull request Sep 26, 2017
Merged
@topper-123 topper-123 deleted the categorical_docs branch September 26, 2017 09:39
alanbato pushed a commit to alanbato/pandas that referenced this pull request Nov 10, 2017
@topper-123 @alanbato
DOC: fixed errors in doc string for Categorical + cleanup (pandas-dev…
eb428df 
…#17655)
No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017
@topper-123 @No-Stream
DOC: fixed errors in doc string for Categorical + cleanup (pandas-dev…
9246473 
…#17655)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jorisvandenbossche jorisvandenbossche jorisvandenbossche approved these changes

Assignees
No one assigned
Labels
Docs
Projects
None yet
Milestone
0.21.0
Development

Successfully merging this pull request may close these issues.
3 participants
@topper-123 @pep8speaks @jorisvandenbossche