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

BUG: Mixed period cannot be displayed with ValueError #12615

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@sinhrks
Member

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 this to the 0.18.1 milestone Mar 14, 2016

@sinhrks sinhrks added the Bug label Mar 14, 2016

@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Mar 14, 2016

Contributor

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)

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)

@codecov-io

This comment has been minimized.

Show comment
Hide comment
@codecov-io

codecov-io Mar 15, 2016

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.

codecov-io commented Mar 15, 2016

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

This comment has been minimized.

Show comment
Hide comment
@sinhrks

sinhrks Mar 16, 2016

Member

OK, changed to raise IncompatibleFrequency and fixed corresponding tests.

Member

sinhrks commented Mar 16, 2016

OK, changed to raise IncompatibleFrequency and fixed corresponding tests.

@@ -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):

This comment has been minimized.

@jreback

jreback Mar 16, 2016

Contributor

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

@jreback

jreback Mar 16, 2016

Contributor

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

This comment has been minimized.

@sinhrks

sinhrks Mar 16, 2016

Member

OK, added.

@sinhrks

sinhrks Mar 16, 2016

Member

OK, added.

@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Mar 16, 2016

Contributor

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

Contributor

jreback commented Mar 16, 2016

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

@jreback

View changes

Show outdated Hide outdated doc/source/whatsnew/v0.18.1.txt Outdated
@jreback

View changes

Show outdated Hide outdated doc/source/whatsnew/v0.18.1.txt Outdated
@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Mar 16, 2016

Contributor

lgtm. ping on green

Contributor

jreback commented Mar 16, 2016

lgtm. ping on green

@jreback jreback closed this in 0f3a7b8 Mar 17, 2016

@jreback

This comment has been minimized.

Show comment
Hide comment
@jreback

jreback Mar 17, 2016

Contributor

thanks @sinhrks

Contributor

jreback commented Mar 17, 2016

thanks @sinhrks

jreback added a commit that referenced this pull request Mar 17, 2016

@sinhrks sinhrks deleted the sinhrks:period_format branch Mar 19, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment