CLN: tslibs typing, avoid private funcs #34195
Conversation
jorisvandenbossche
left a comment
jorisvandenbossche
left a comment
There was a problem hiding this comment.
Generally looks good, just two small comments
pandas/_libs/tslibs/period.pyx
Outdated
|
|
||
| def strftime(self, fmt: str) -> str: | ||
| """ | ||
| r""" |
There was a problem hiding this comment.
Is there a reason this is needed? (the docs seem to render fine: https://pandas.pydata.org/docs/dev/reference/api/pandas.Period.strftime.html#pandas.Period.strftime)
There was a problem hiding this comment.
there are \( in there that are invalid escapes
There was a problem hiding this comment.
It might be needed for sphinx (no idea), because as shown, it renders good. Did you check that it still renders fine with this change?
There was a problem hiding this comment.
I suppose this is copied from the stdlib docs, and there those escape is also used: https://raw.githubusercontent.com/python/cpython/3.8/Doc/library/datetime.rst
There was a problem hiding this comment.
I mainly notice this because the syntax highlighter (sublime text) marks it
There was a problem hiding this comment.
@jorisvandenbossche do you feel strongly that this should be reverted? i care about this less than the rest of the PR, so would be OK with that
There was a problem hiding this comment.
I don't care about whether this should be reverted or not, but if you change it, you need to build the docs to check that it actually works (because just based on guessing, I wouldn't know whether this change is correct or not)
pandas/_libs/tslibs/period.pyx
Outdated
|
|
||
| def strftime(self, fmt: str) -> str: | ||
| """ | ||
| r""" |
There was a problem hiding this comment.
I don't care about whether this should be reverted or not, but if you change it, you need to build the docs to check that it actually works (because just based on guessing, I wouldn't know whether this change is correct or not)
|
Reverted the "r"; test failure is in feather |
|
updated+green |
4 participants
No description provided.