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

BUG: Mixed period cannot be displayed with ValueError #12615

Closed
wants to merge 1 commit into from
Closed

BUG: Mixed period cannot be displayed with ValueError #12615

wants to merge 1 commit into from

Conversation

sinhrks
Copy link
Member

@sinhrks sinhrks commented Mar 14, 2016

  • closes #xxxx (not reported)
  • tests added / passed
  • passes git diff upstream/master | flake8 --diff
  • whatsnew entry
# we can create Series contains Periods with mixed freq
s = pd.Series([pd.Period('2011-01', freq='M'), pd.Period('2011-02-01', freq='D')])

# print the created Series raises ValueError
print(s)
# ValueError: Input has different freq=D from PeriodIndex(freq=M)

@sinhrks sinhrks added Output-Formatting __repr__ of pandas objects, to_string Period Period data type labels Mar 14, 2016
@sinhrks sinhrks added this to the 0.18.1 milestone Mar 14, 2016
@sinhrks sinhrks added the Bug label Mar 14, 2016
@jreback
Copy link
Contributor

jreback commented Mar 14, 2016

I would change the exception that PeriodIndex raises to maybe: IncompatibileFrequency(ValueError) and catch that. Its a more informative error I think. (its in src/periods.pyx/extract_ordinals)

@sinhrks sinhrks force-pushed the period_format branch 4 times, most recently from 6da8f8d to 6b3d2a1 Compare March 15, 2016 21:13
@codecov-io
Copy link

Current coverage is 88.64%

Merging #12615 into master will not affect coverage as of b12fd75

@@            master   #12615   diff @@
=======================================
  Files          275      275       
  Stmts       133688   133716    +28
  Branches         0        0       
  Methods          0        0       
=======================================
+ Hit         118511   118539    +28
  Partial          0        0       
  Missed       15177    15177

Review entire Coverage Diff as of b12fd75

Uncovered Suggestions

  1. +0.07% via ...s/io/sas/sas7bdat.py#30...121
  2. +0.03% via ...s/io/sas/sas7bdat.py#531...569
  3. +0.03% via ...as/tools/plotting.py#633...664
  4. See 7 more...

Powered by Codecov. Updated on successful CI builds.

@sinhrks
Copy link
Member Author

sinhrks commented Mar 16, 2016

OK, changed to raise IncompatibleFrequency and fixed corresponding tests.

jreback
jreback reviewed Mar 16, 2016
View reviewed changes
pandas/src/period.pyx
@@ -627,6 +628,11 @@ cdef ndarray[int64_t] localize_dt64arr_to_period(ndarray[int64_t] stamps,
_DIFFERENT_FREQ = "Input has different freq={1} from Period(freq={0})"
_DIFFERENT_FREQ_INDEX = "Input has different freq={1} from PeriodIndex(freq={0})"


class IncompatibleFrequency(ValueError):
Copy link
Contributor

Choose a reason for hiding this comment

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

if its helpful you can import this into pandas.tseries.common. can do that in the future.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, added.

@jreback
Copy link
Contributor

jreback commented Mar 16, 2016

lgtm. let me know if you want to change the import now or later.

jreback
jreback reviewed Mar 16, 2016
View reviewed changes
doc/source/whatsnew/v0.18.1.txt
@@ -44,7 +44,8 @@ Enhancements
API changes
~~~~~~~~~~~


- ``Period`` and ``PeriodIndex`` now raises ``IncompatibleFrequency`` error
Copy link
Contributor

Choose a reason for hiding this comment

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

btw, usually just make these 1 line as the formatter will wrap it (and I think create a weird space otherwise)

@sinhrks sinhrks force-pushed the period_format branch 3 times, most recently from ef04e9f to c4737ae Compare March 16, 2016 14:45
@jreback
Copy link
Contributor

jreback commented Mar 16, 2016

lgtm. ping on green

@jreback jreback closed this in 0f3a7b8 Mar 17, 2016
@jreback
Copy link
Contributor

jreback commented Mar 17, 2016

thanks @sinhrks

jreback added a commit that referenced this pull request Mar 17, 2016
@jreback
TST: compat of tests xref #12615 on windows
2a53113
@sinhrks sinhrks deleted the period_format branch March 19, 2016 20:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Bug Output-Formatting __repr__ of pandas objects, to_string Period Period data type
Projects
None yet
Milestone
0.18.1
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sinhrks @jreback @codecov-io