Skip to content

CLN: reorg pandas.tseries #16040

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 12 commits into from
Apr 18, 2017
Merged

CLN: reorg pandas.tseries #16040

merged 12 commits into from
Apr 18, 2017

Conversation

jreback
Copy link
Contributor

@jreback jreback commented Apr 18, 2017

xref #13634

@jreback jreback added Clean Period Period data type Timedelta Timedelta data type Datetime Datetime data dtype labels Apr 18, 2017
@jreback jreback added this to the 0.20.0 milestone Apr 18, 2017
@jreback
Copy link
Contributor Author

jreback commented Apr 18, 2017

this finishes the api cleanup and moves everything but:

  • freqencies, holidays, offsets
  • util, and plotting (which are deprecated)

from pandas.tseries.

all of the indexes code is now actually in pandas.core.indexes (tests were moved previously).

@codecov
Copy link

codecov bot commented Apr 18, 2017 •
edited
Loading

Codecov Report

Merging #16040 into master will increase coverage by 0.02%.
The diff coverage is 99.34%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16040      +/-   ##
==========================================
+ Coverage   90.76%   90.79%   +0.02%     
==========================================
  Files         155      156       +1     
  Lines       50484    50489       +5     
==========================================
+ Hits        45824    45839      +15     
+ Misses       4660     4650      -10
Flag Coverage Δ
#multiple 88.55% <99.34%> (+0.02%) ⬆️
#single 40.42% <47.71%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/tseries/api.py 100% <ø> (ø) ⬆️
pandas/core/tools/timedeltas.py 98.33% <ø> (ø)
pandas/compat/pickle_compat.py 69.51% <ø> (ø) ⬆️
pandas/core/datetools.py 86.84% <100%> (ø) ⬆️
pandas/plotting/_timeseries.py 61.78% <100%> (ø) ⬆️
pandas/core/tools/numeric.py 100% <100%> (ø)
pandas/core/indexes/api.py 98.75% <100%> (+0.06%) ⬆️
pandas/core/resample.py 94.45% <100%> (ø)
pandas/plotting/_converter.py 65.35% <100%> (+1.81%) ⬆️
pandas/core/indexes/base.py 96.19% <100%> (ø) ⬆️
... and 24 more

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 d16cce8...82496f2. Read the comment docs.

jorisvandenbossche
jorisvandenbossche reviewed Apr 18, 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.

We also need to think of something regarding the API docs. Currently that uses pandas.tseries.resample (eg http://pandas.pydata.org/pandas-docs/stable/generated/pandas.tseries.resample.Resampler.aggregate.html)

That will have to change, meaning all doc links won't work anymore (not that big of a problem I think), but maybe we should think about a more long term option, and also expose this somewhere less nested that will not change? (but not sure where, as I would not add it top-level)

doc/source/conf.py Outdated
@@ -231,7 +231,7 @@
moved_api_pages = [
'pandas.core.common.isnull', 'pandas.core.common.notnull', 'pandas.core.reshape.get_dummies',
'pandas.tools.merge.concat', 'pandas.tools.merge.merge', 'pandas.tools.pivot.pivot_table',
'pandas.tseries.tools.to_datetime', 'pandas.io.clipboard.read_clipboard', 'pandas.io.excel.ExcelFile.parse',
Copy link
Member

Choose a reason for hiding this comment

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

this shouldn't be changed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@jreback
Copy link
Contributor Author

jreback commented Apr 18, 2017

We also need to think of something regarding the API docs. Currently that uses pandas.tseries.resample (eg http://pandas.pydata.org/pandas-docs/stable/generated/pandas.tseries.resample.Resampler.aggregate.html)

this just changes to pandas.core.resample so I don't see a problem.

@jorisvandenbossche
Copy link
Member

Yes, but that means that all our online pages change url

@jorisvandenbossche
Copy link
Member

You created here the core.tools module, which seems like a good idea, but then it may make sense to also put to_numeric there for consistency?

@jreback
Copy link
Contributor Author

jreback commented Apr 18, 2017

You created here the core.tools module, which seems like a good idea, but then it may make sense to also put to_numeric there for consistency?

yeah was debating this. ok will move it.

Yes, but that means that all our online pages change url

and? lots of thinks moved.

@jorisvandenbossche
Copy link
Member

and? lots of thinks moved.

yes, but up to now (I mean the reorg the last days), no 'public' things. And Resampler is not really public, apart from that it is used in the docs.

@jreback
Copy link
Contributor Author

jreback commented Apr 18, 2017

can we do a re-direct?

@jreback jreback merged commit c8dafb5 into pandas-dev:master Apr 18, 2017
@jorisvandenbossche
Copy link
Member

can we do a re-direct?

yes, that is certainly possible.

My concern is only that the urls currently depend a bit on the 'implementation details' in core (as one of the reasons of this reorg and the more explicit saying that core is private, is to be able to refactor internally). So just wondering whether it would make sense to expose eg GroupBy, Rolling, Resampler in eg pandas.core directly, not as a public API, as no user should directly use those objects, but to have a stable doc url independent of where or how in core it is implemented.

Although chance is high that the pandas.core.groupby.GroupBy, pandas.core.window.Rolling, pandas.core.resample.Resampler paths will not directly be changed again, so maybe not that important.

@jreback
Copy link
Contributor Author

jreback commented Apr 19, 2017

could u do some kind ornate shim for the actual links?

maybe in conf.py make a namespace like

pandas.doclinks or something then just assign the classes there ?

analyticalmonk pushed a commit to analyticalmonk/pandas that referenced this pull request Apr 20, 2017
@jreback
CLN: reorg pandas.tseries (pandas-dev#16040)
1d2a0c9 
* CLN: move pandas/tseries/resample.py -> pandas/core/resample.py

closes pandas-dev#13634

* CLN: move pandas.tseries.period -> pandas.core.indexes.period

* CLN: move pandas.tseries.tdi -> pandas.core.indexes.timedeltas

* CLN: move pandas.tseries.base -> pandas.core.indexes.datetimelike

* CLN: pandas.tseries.common -> pandas.core.indexes.accessors

* CLN: move pandas.tseries.index -> pandas.core.indexes.datetimes

* CLN: move pandas.tseries.timedeltas, pandas.tseries.tools -> pandas.core.tools

* move to_numeric to pandas.core.tools.numeric
@jorisvandenbossche
Copy link
Member

I am not sure something like that would work. As sphinx has to actually import it to get the docstring (I think, not sure how it works under the hood)

@EdAbati EdAbati mentioned this pull request Oct 7, 2020
Closed
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
Clean Datetime Datetime data dtype Period Period data type Timedelta Timedelta data type
Projects
None yet
Milestone
0.20.0
Development

Successfully merging this pull request may close these issues.
2 participants
@jreback @jorisvandenbossche