Performance of maybe_box_datetimelike #30520 #30531
Conversation
|
Hello @ivan-vasilev! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-01-01 23:40:02 UTC |
jreback
left a comment
jreback
left a comment
There was a problem hiding this comment.
pls add an asv and a whatsnew note
i suppose could change for Timedelta as well
pandas/core/common.py
Outdated
|
|
||
| if isinstance(value, (np.datetime64, datetime)): | ||
| if isinstance(value, (np.datetime64, datetime)) \ | ||
| and not isinstance(value, tslibs.Timestamp): |
|
See my comment in the issue: #30520 (comment). Is this really the appropriate place for the performance fix? Or should this be done in the |
simonjayhawkins
changed the title
|
linting issue should be fixable by running |
jreback
left a comment
jreback
left a comment
There was a problem hiding this comment.
some comments, pls add a whatsnew (perf section), be generic about it. e.g. elaborate on what a user would see at a high level, e.g. you have a Series and do xyz to it, now its faster. also add this example as an asv.
| lib.maybe_convert_numeric(data, set(), coerce_numeric=False) | ||
|
|
||
|
|
||
| from .pandas_vb_common import setup # noqa: F401 isort:skip |
pandas/core/common.py
Outdated
| and obj.stop is None | ||
| and obj.step is None | ||
| isinstance(obj, slice) | ||
| and obj.start is None |
There was a problem hiding this comment.
can you avoid other whitespace changes
|
this is subsumed by #30676 (though if you still think there is a perf benefit after that certainly ping). |
6 participants
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffmaybe_box_datetimelikeavoids unnecessary conversion ofvaluetotslibs.Timestamportslibs.Timedeltaifvalueis already an instance oftslibs.Timestamportslibs.Timedelta.