Skip to content

day_of_week #26094

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

Closed

Conversation

Violet-XiaoWeiHuang
Copy link

@Violet-XiaoWeiHuang Violet-XiaoWeiHuang commented Apr 15, 2019

@pep8speaks
Copy link

Hello @Violet-XiaoWeiHuang! Thanks for opening this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 50:13: E117 over-indented

@mroeschke
Copy link
Member

As mentioned #25924 (comment), dayofweek needs to be deprecated first. This can't just be replaced.

@codecov
Copy link

codecov bot commented Apr 15, 2019

Codecov Report

Merging #26094 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26094      +/-   ##
==========================================
- Coverage   91.95%   91.94%   -0.01%     
==========================================
  Files         175      175              
  Lines       52443    52445       +2     
==========================================
- Hits        48224    48222       -2     
- Misses       4219     4223       +4
Flag Coverage Δ
#multiple 90.5% <100%> (ø) ⬆️
#single 40.75% <100%> (-0.14%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/period.py 92.04% <ø> (ø) ⬆️
pandas/core/indexes/datetimes.py 96.4% <ø> (ø) ⬆️
pandas/core/arrays/period.py 98.54% <100%> (ø) ⬆️
pandas/core/arrays/datetimes.py 97.8% <100%> (ø) ⬆️
pandas/io/gbq.py 75% <0%> (-12.5%) ⬇️
pandas/core/frame.py 96.79% <0%> (-0.12%) ⬇️

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 4e7e7f0...e8bf846. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Apr 15, 2019

Codecov Report

Merging #26094 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #26094      +/-   ##
==========================================
- Coverage   91.95%   91.94%   -0.01%     
==========================================
  Files         175      175              
  Lines       52443    52445       +2     
==========================================
- Hits        48224    48222       -2     
- Misses       4219     4223       +4
Flag Coverage Δ
#multiple 90.5% <100%> (ø) ⬆️
#single 40.75% <100%> (-0.14%) ⬇️
Impacted Files Coverage Δ
pandas/core/indexes/period.py 92.04% <ø> (ø) ⬆️
pandas/core/indexes/datetimes.py 96.4% <ø> (ø) ⬆️
pandas/core/arrays/period.py 98.54% <100%> (ø) ⬆️
pandas/core/arrays/datetimes.py 97.8% <100%> (ø) ⬆️
pandas/io/gbq.py 75% <0%> (-12.5%) ⬇️
pandas/core/frame.py 96.79% <0%> (-0.12%) ⬇️

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 4e7e7f0...e8bf846. Read the comment docs.

@gfyoung gfyoung added API Design Datetime Datetime data dtype labels Apr 15, 2019
self.ts.dayofweek
self.ts.day_of_week

def time_day_of_week(self, tz, freq):
Copy link
Contributor

Choose a reason for hiding this comment

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

indent error here

@@ -44,7 +44,10 @@ def time_tz(self, tz, freq):
self.ts.tz

def time_dayofweek(self, tz, freq):
Copy link
Contributor

Choose a reason for hiding this comment

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

you can remove the original one

@@ -527,7 +527,8 @@ generated/pandas.DatetimeIndex.ceil,../reference/api/pandas.DatetimeIndex.ceil
generated/pandas.DatetimeIndex.date,../reference/api/pandas.DatetimeIndex.date
generated/pandas.DatetimeIndex.day,../reference/api/pandas.DatetimeIndex.day
generated/pandas.DatetimeIndex.day_name,../reference/api/pandas.DatetimeIndex.day_name
generated/pandas.DatetimeIndex.dayofweek,../reference/api/pandas.DatetimeIndex.dayofweek
generated/pandas.DatetimeIndex.dayofweek,../reference/api/pandas.DatetimeIndex.day_of_week
generated/pandas.DatetimeIndex.day_of_week,../reference/api/pandas.DatetimeIndex.day_of_week
Copy link
Contributor

Choose a reason for hiding this comment

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

i don't think you need to add the new one, we don't have original pages for it

@@ -60,6 +60,7 @@ Properties
Timestamp.asm8
Timestamp.day
Timestamp.dayofweek
Copy link
Contributor

Choose a reason for hiding this comment

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

you can remove the original from the docs

@@ -138,7 +138,7 @@ Bug Fixes
- Bug where ``Panel.from_dict`` does not set dtype when specified (:issue:`10058`)
- Bug in ``Index.union`` raises ``AttributeError`` when passing array-likes. (:issue:`10149`)
- Bug in ``Timestamp``'s' ``microsecond``, ``quarter``, ``dayofyear``, ``week`` and ``daysinmonth`` properties return ``np.int`` type, not built-in ``int``. (:issue:`10050`)
- Bug in ``NaT`` raises ``AttributeError`` when accessing to ``daysinmonth``, ``dayofweek`` properties. (:issue:`10096`)
- Bug in ``NaT`` raises ``AttributeError`` when accessing to ``daysinmonth``, ``dayofweek``, ``day_of_week`` properties. (:issue:`10096`)
Copy link
Contributor

Choose a reason for hiding this comment

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

don't change the original what's new notes

"""
base, mult = get_freq_code(self.freq)
return pweekday(self.ordinal, base)

@property
def dayofweek(self):
Copy link
Contributor

Choose a reason for hiding this comment

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

just do

dayofweek = day_of_week

@jreback
Copy link
Contributor

jreback commented May 12, 2019

can you merge master and update per comments

@jreback
Copy link
Contributor

jreback commented Jun 8, 2019

closing as stale. this is pretty close, so ping if you'd like to continue.

@jreback jreback closed this Jun 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Design Datetime Datetime data dtype
Projects
None yet
Development

Successfully merging this pull request may close these issues.

API: timeseries accessors naming convention
5 participants