Skip to content

DOC: Improved doc string for IntervalIndex + related changes #17706

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 2 commits into from
Sep 28, 2017
Merged

DOC: Improved doc string for IntervalIndex + related changes #17706

merged 2 commits into from
Sep 28, 2017
+96 −18

Conversation

@topper-123
Copy link
Contributor

@topper-123 topper-123 commented Sep 28, 2017

  • [x ] passes git diff upstream/master -u -- "*.py" | flake8 --diff

This PR has Improvements for the doc string for IntervalIndex + related changes

jorisvandenbossche
jorisvandenbossche reviewed Sep 28, 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.

Cool!

pandas/core/indexes/interval.py Outdated
closed='right', dtype='interval[int64]')
It may also be constructed using one of the constructor
methods :meth:`IntervalIndex.from_arrays``,
Copy link
Member

@jorisvandenbossche jorisvandenbossche Sep 28, 2017

Choose a reason for hiding this comment

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

double -> single closing backtick

pandas/core/indexes/interval.py Outdated
It may also be constructed using one of the constructor
methods :meth:`IntervalIndex.from_arrays``,
:meth:`IntervalIndex.from_breaks``, :meth:`IntervalIndex.from_intervals`
and :meth:`IntervalIndex.from_tuples``.
Copy link
Member

@jorisvandenbossche jorisvandenbossche Sep 28, 2017

Choose a reason for hiding this comment

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

and the above two lines as well

pandas/core/indexes/interval.py
>>> pd.IntervalIndex.from_tuples([(0, 1), (1,2)])
IntervalIndex([(0, 1], (1, 2]],
closed='right', dtype='interval[int64]')
Copy link
Member

@jorisvandenbossche jorisvandenbossche Sep 28, 2017

Choose a reason for hiding this comment

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

We could add here 'See also' to the other from_ constructors

pandas/core/indexes/interval.py Outdated
Index
Interval : A bounded slice-like interval
interval_range : Function to create a fixed frequency IntervalIndex
IntervalIndex.from_arrays : Construct an IntervalIndex from a a left and
Copy link
Member

@jorisvandenbossche jorisvandenbossche Sep 28, 2017

Choose a reason for hiding this comment

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

'a a'

pandas/core/indexes/interval.py Outdated
right array
IntervalIndex.from_breaks : Construct an IntervalIndex from an array of
splits
IntervalIndex.from_intervals : Construct an IntervalIndex from a array of
Copy link
Member

@jorisvandenbossche jorisvandenbossche Sep 28, 2017

Choose a reason for hiding this comment

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

a -> an

@jreback jreback added the Docs label Sep 28, 2017
@codecov
Copy link

codecov bot commented Sep 28, 2017 •
edited
Loading

Codecov Report

Merging #17706 into master will increase coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #17706      +/-   ##
==========================================
+ Coverage   91.24%   91.25%   +<.01%     
==========================================
  Files         163      163              
  Lines       49766    49765       -1     
==========================================
  Hits        45411    45411              
+ Misses       4355     4354       -1
Flag Coverage Δ
#multiple 89.04% <ø> (+0.01%) ⬆️
#single 40.34% <ø> (-0.06%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/interval.py 92.85% <ø> (ø) ⬆️
pandas/io/gbq.py 25% <0%> (-58.34%) ⬇️
pandas/core/sparse/array.py 91.3% <0%> (-0.29%) ⬇️
pandas/core/frame.py 97.73% <0%> (-0.1%) ⬇️
pandas/core/indexes/datetimes.py 95.44% <0%> (-0.1%) ⬇️
pandas/io/pytables.py 92.79% <0%> (ø) ⬆️
pandas/plotting/_converter.py 65.2% <0%> (+1.81%) ⬆️

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 074b485...7dfad50. Read the comment docs.

@topper-123 topper-123 force-pushed the IntervalIndex_doc_string branch from ef3d7c0 to 2deb7ac Compare September 28, 2017 15:14
@pep8speaks
Copy link

pep8speaks commented Sep 28, 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 28, 2017 at 15:55 Hours UTC

@topper-123 topper-123 force-pushed the IntervalIndex_doc_string branch from 2deb7ac to 5615792 Compare September 28, 2017 15:24
@topper-123
Copy link
Contributor Author

topper-123 commented Sep 28, 2017

Ok, all comments taken care of.

@topper-123
Copy link
Contributor Author

topper-123 commented Sep 28, 2017 •
edited
Loading

Hi @jorisvandenbossche . I've found, that the output string from the examples in the from_* methods is wrong, and I'm guessing they are from an older version of IntervalIndex. The second commit proposal is regarding that.

In addition, the time-based examples for pd.interval_range don't work. I'l submit a issue regarding that as it must be a code issue. Edit : filed issue as #17707 on this

@topper-123 topper-123 force-pushed the IntervalIndex_doc_string branch from 9c7883f to 7dfad50 Compare September 28, 2017 15:55
@jorisvandenbossche
Copy link
Member

jorisvandenbossche commented Sep 28, 2017

I'm guessing they are from an older version of IntervalIndex

Yes, the repr was changes during the PR process of adding the IntervalIndex, so not examples were updated for that

@jorisvandenbossche jorisvandenbossche merged commit cc58b84 into pandas-dev:master Sep 28, 2017
@jorisvandenbossche
Copy link
Member

jorisvandenbossche commented Sep 28, 2017

@topper-123 Thanks a lot!

@topper-123 topper-123 mentioned this pull request Sep 29, 2017
Merged
@topper-123 topper-123 deleted the IntervalIndex_doc_string branch October 9, 2017 21:00
alanbato pushed a commit to alanbato/pandas that referenced this pull request Nov 10, 2017
@topper-123 @alanbato
DOC: Improved doc string for IntervalIndex + related changes (pandas-…
50ab764 
…dev#17706)
No-Stream pushed a commit to No-Stream/pandas that referenced this pull request Nov 28, 2017
@topper-123 @No-Stream
DOC: Improved doc string for IntervalIndex + related changes (pandas-…
b6265d3 
…dev#17706)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jorisvandenbossche jorisvandenbossche jorisvandenbossche left review comments

Assignees

No one assigned

Labels

Docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@topper-123 @pep8speaks @jorisvandenbossche @jreback