Skip to content

Conversation

ShaharNaveh
Copy link
Contributor

@ShaharNaveh ShaharNaveh commented Apr 8, 2020


When running pytest -q --cache-clear pandas/tests/ -k "period" some tests in pandas/tests/indexing/ are failing, but when running pytest -q --cache-clear pandas/tests/indexing/ all the tests are passing (on my local machine), so I figured and debug from here, thoughts?

@jbrockmendel
Copy link
Member

thoughts?

we cant rely on grepping to get all the relevant tests. i recommend pytest pandas/tests --skip-slow --skip-db, takes about 15 minutes for me.

Look at the failing tests, choose one and start executing it step-by-step in the REPL

off = "QS"
rng = pd.date_range("01-Jan-2012", periods=8, freq=off)

>>> prng = rng.to_period()   # <-- raises, so recurse into the `to_period` method

)

# Using simple filter because we are not checking for the warning here
warnings.simplefilter("ignore", UserWarning)
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the warning from? this would not be set like this
use assert_produces_warning(None):
warnings.simplefiler....

if you really need to

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what is the warning from? this would not be set like this

The warning raises when a user is attempting to convert a date_range to period, if the date_range have a timezone it will drop it in the conversion, the warning let the user know about that:

if self.tz is not None:
warnings.warn(
"Converting to PeriodArray/Index representation "
"will drop timezone information.",
UserWarning,
)

use assert_produces_warning(None):
warnings.simplefiler....

if you really need to

Sure! will fix.

Copy link
Contributor

Choose a reason for hiding this comment

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

i c, ok then

@jreback jreback added Frequency DateOffsets Period Period data type labels Apr 10, 2020
@jreback jreback added this to the 1.1 milestone Apr 10, 2020
@jreback
Copy link
Contributor

jreback commented Apr 10, 2020

looks good, can you add a whatsnew note in 1.1 in the datetime section for bug fixes. ping on green.

@ShaharNaveh ShaharNaveh requested a review from jreback April 11, 2020 14:39
@ShaharNaveh
Copy link
Contributor Author

ping @jreback

@jreback jreback merged commit 13dc13f into pandas-dev:master Apr 12, 2020
@jreback
Copy link
Contributor

jreback commented Apr 12, 2020

thanks @MomIsBestFriend

@ShaharNaveh ShaharNaveh deleted the BUG-DatetimeIndex-to_period branch April 13, 2020 08:48
# https://github.com/pandas-dev/pandas/issues/33358
if res is None:
base, stride = libfrequencies._base_and_stride(freq)
res = f"{stride}{base}"
Copy link
Member

Choose a reason for hiding this comment

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

@MomIsBestFriend why cant we just do res = freq here? (im trying to clean up usages of base_and_stride)

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

Labels

Frequency DateOffsets Period Period data type

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DatetimeIndex.to_period with freq

3 participants