pandas icon indicating copy to clipboard operation
pandas copied to clipboard

Published 20 hours ago • verified publisher icon pandas-dev

Warn when creating `Period` with a string that includes timezone information

Open joooeey opened this issue 2 years ago • 1 comments

  • [x] closes #47005
  • [x] Tests added and passed if fixing a bug or adding a new feature
  • [x] All code checks passed.
  • [ ] Added type annotations to new arguments/methods/functions. -- no new arguments/methods/functions.
  • [x] Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

~~Note on the new DateTimeWarning~~

~~I added the DateTimeWarning for this specific case and the warnings raised in #22549 because the commonly used UserWarning is often not appropriate. There are lots of conceivable situations where this warning may be raised due to design choices in Pandas as opposed to user error. DateTimeWarning inherits from UserWarning to remain backwards compatible. This trail of thought applies to much of the datetime functionality in Pandas but that's beyond the scope of this PR.~~

joooeey avatar Aug 06 '22 13:08 joooeey

we don't need a special warning here just use the standard UserWarning

Done.

I was just thinking it's not good UX if we raise UserWarning for things that are not necessarily mistakes by the user which I've seen a few examples. But then there's not really a better Warning in the built-in Exceptions except perhaps Warning.

joooeey avatar Aug 06 '22 14:08 joooeey

@jreback can this be merged? I implemented your requested change the day I opened the merge request.

joooeey avatar Aug 23 '22 14:08 joooeey

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

github-actions[bot] avatar Sep 29 '22 00:09 github-actions[bot]

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

I'm still interested in working on this but I'm swamped with other things right now. I'll get to it later this year.

joooeey avatar Sep 29 '22 07:09 joooeey

Hi @joooeey - is this still active? Shall we close for now and can revisit when you have time?

MarcoGorelli avatar Dec 08 '22 09:12 MarcoGorelli

Hi @MarcoGorelli! I might have time to do all the changes from my side this weekend or on Tuesday. If I don't get to it, I'll close the PR next week (for the time being).

joooeey avatar Dec 09 '22 07:12 joooeey

Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen.

mroeschke avatar Jan 16 '23 19:01 mroeschke

I think the "W" parts of this have recently been fixed separately, would be good to see this revived. @joooeey up for it?

jbrockmendel avatar Mar 04 '23 22:03 jbrockmendel

In principle, I'm up for it. I've kept this on my backlog all this time. But I can't give a timeline.

joooeey avatar Mar 06 '23 08:03 joooeey