Skip to content
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

DEPR: camelCase in offsets, get_offset #30340

Merged
merged 9 commits into from Dec 27, 2019
Merged

DEPR: camelCase in offsets, get_offset #30340

merged 9 commits into from Dec 27, 2019

Conversation

jbrockmendel
Copy link
Member

@jbrockmendel jbrockmendel added Deprecate Functionality to remove in pandas Frequency DateOffsets labels Dec 20, 2019
@jbrockmendel
Copy link
Member Author

@datapythonista the good news is im able to reproduce this locally with make doc like you suggested, the bad is that I have no idea why I'm getting a bunch of these complaints:

/home/vsts/work/1/s/doc/source/reference/api/pandas.tseries.offsets.BQuarterBegin.isAnchored.rst: WARNING: document isn't included in any toctree

ideas?

@datapythonista
Copy link
Member

We fixed a lot of these in the past, but I'm not sure if we ever understood the problem. At least I don't think I did.

I think what we do is to add the docs being reported to a hidden autosummary. See: https://github.com/pandas-dev/pandas/blame/master/doc/source/reference/index.rst#L44

@jorisvandenbossche probably knows more

@jorisvandenbossche
Copy link
Member

BQuarterBegin.isAnchored.rst: WARNING: document isn't included in any toctree

It's because you removed those from the api doc pages, while they are still generated automatically from the class docstring. But so they also need to be included somewhere in another autosummary that adds them into the toctree. So like Marc said, we use hidden autosummaries to solve this. There should be some other examples in the api docs of that (look at the bottom of those pages).

@jorisvandenbossche
Copy link
Member

See the "hidden" text in doc/source/reference/index.rst (https://github.com/pandas-dev/pandas/blob/master/doc/source/reference/index.rst)

@jbrockmendel
Copy link
Member Author

thank you both

@jbrockmendel
Copy link
Member Author

@jorisvandenbossche my attempts at mimicing the examples in offset_frequency.rst haven't worked. in this commit i used "api.offsets.Foo.onOffset" but itt also didn't work with "Foo.onOffset", "api/Foo.onOffset", "pandas.tseries.offsets.Foo.onOffset"

@jorisvandenbossche
Copy link
Member

The toctree needs the path (which becomes the url, eg https://dev.pandas.io/docs/reference/api/pandas.tseries.offsets.DateOffset.isAnchored.html), so you need something like api/pandas.tseries.offsets.DateOffset.isAnchored. However, if you do it on this page, it might need to be ../api/pandas... since it is one level down in the hierarchy.

@jbrockmendel
Copy link
Member Author

@jorisvandenbossche see newest commit where i tried just about every variant I could think of, including putting them in a different file where we already use this pattern.

jorisvandenbossche
jorisvandenbossche reviewed Dec 21, 2019
View reviewed changes
doc/source/reference/index.rst Outdated
@@ -61,6 +61,546 @@ public functions related to data types in pandas.
api/pandas.Series.from_array
api/pandas.Series.imag
api/pandas.Series.real
..api/pandas.tseries.offsets.DateOffset.isAnchored
Copy link
Member

Choose a reason for hiding this comment

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

if you put it here it doesn't need the '..', see the lines above

Copy link
Member

Choose a reason for hiding this comment

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

OK, didn't see what you did below :)

jorisvandenbossche
jorisvandenbossche reviewed Dec 23, 2019
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.

On the actual PR, is there any difference between get_offset and to_offset that the user should be aware of? (the implementation is not exactly the same)

pandas/tseries/offsets.py
return self.n == 1

def onOffset(self, dt):
warnings.warn(
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a docstring with a ..deprecated:: directive?

pandas/tseries/frequencies.py
@@ -185,10 +186,29 @@ def get_offset(name: str) -> DateOffset:
"""
Return DateOffset object associated with rule name.

.. deprecated:: 1.0.0

Copy link
Member

Choose a reason for hiding this comment

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

Can you add here that to_offset should be used instead?

@jbrockmendel
Copy link
Member Author

is there any difference between get_offset and to_offset that the user should be aware of?

I dont think get_offset is supported in a meaningful enough way for this to make sense. e.g. i just found the example in the docstring was wrong

jreback
jreback reviewed Dec 27, 2019
View reviewed changes
pandas/tseries/frequencies.py
FutureWarning,
stacklevel=2,
)
return _get_offset(name)
Copy link
Contributor

Choose a reason for hiding this comment

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

can this call to_offset instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, we still need _get_offset internally, just officially telling users to stay away

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, would rename maybe if you can (followon ok)

jreback
jreback requested changes Dec 27, 2019
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

comment

jreback
jreback reviewed Dec 27, 2019
View reviewed changes
Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

did you add a deprecation note in whatsnew?

@jbrockmendel
Copy link
Member Author

did you add a deprecation note in whatsnew?

yes

@jreback jreback added this to the 1.0 milestone Dec 27, 2019
@jreback jreback merged commit 1d36851 into pandas-dev:master Dec 27, 2019
@jreback
Copy link
Contributor

jreback commented Dec 27, 2019

great thanks.

@jorisvandenbossche jorisvandenbossche mentioned this pull request Dec 27, 2019
Closed
@jbrockmendel jbrockmendel deleted the depr-get_offset branch December 27, 2019 20:58
AlexKirko pushed a commit to AlexKirko/pandas that referenced this pull request Dec 29, 2019
@jbrockmendel @AlexKirko
DEPR: camelCase in offsets, get_offset (pandas-dev#30340)
5481f51
keechongtan added a commit to keechongtan/pandas that referenced this pull request Dec 29, 2019
@keechongtan
Merge remote-tracking branch 'upstream/master' into bug/categorical-i…
c5a7f6e 
…ndexing-1row-df

* upstream/master: (333 commits)
  CI: troubleshoot Web_and_Docs failing (pandas-dev#30534)
  WARN: Ignore NumbaPerformanceWarning in test suite (pandas-dev#30525)
  DEPR: camelCase in offsets, get_offset (pandas-dev#30340)
  PERF: implement scalar ops blockwise (pandas-dev#29853)
  DEPR: Remove Series.compress (pandas-dev#30514)
  ENH: Add numba engine for rolling apply (pandas-dev#30151)
  [ENH] Add to_markdown method (pandas-dev#30350)
  DEPR: Deprecate pandas.np module (pandas-dev#30386)
  ENH: Add ignore_index for df.drop_duplicates (pandas-dev#30405)
  BUG: The setting xrot=0 in DataFrame.hist() doesn't work with by and subplots pandas-dev#30288 (pandas-dev#30491)
  CI: Fix GBQ Tests (pandas-dev#30478)
  Bug groupby quantile listlike q and int columns (pandas-dev#30485)
  ENH: Add ignore_index for df.sort_values and series.sort_values (pandas-dev#30402)
  TYP: Typing hints in pandas/io/formats/{css,csvs}.py (pandas-dev#30398)
  BUG: raise on non-hashable Index name, closes pandas-dev#29069 (pandas-dev#30335)
  Replace "foo!r" to "repr(foo)" syntax pandas-dev#29886 (pandas-dev#30502)
  BUG: preserve EA dtype in transpose (pandas-dev#30091)
  BLD: add check to prevent tempita name error, clsoes pandas-dev#28836 (pandas-dev#30498)
  REF/TST: method-specific files for test_append (pandas-dev#30503)
  marked unused parameters (pandas-dev#30504)
  ...
@mroeschke mroeschke mentioned this pull request Oct 25, 2022
Merged
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

@jreback jreback jreback approved these changes

Assignees
No one assigned
Labels
Deprecate Functionality to remove in pandas Frequency DateOffsets
Projects
None yet
Milestone
1.0.0
Development

Successfully merging this pull request may close these issues.

API: deprecate get_offset, Offset for time rules like '30s'
4 participants
@jbrockmendel @datapythonista @jorisvandenbossche @jreback