CLN: address TODOs #33886
CLN: address TODOs #33886
Conversation
pandas/_libs/lib.pyx
Outdated
| @@ -221,7 +221,7 @@ def is_iterator(obj: object) -> bool: | |||
| return PyIter_Check(obj) | |||
|
|
|||
|
|
|||
| def item_from_zerodim(val: object) -> object: | |||
| cpdef object item_from_zerodim(object val): | |||
There was a problem hiding this comment.
could i guess, i dont think it makes a difference
There was a problem hiding this comment.
on balance i prefer not to mix "inline" with "cpdef"
There was a problem hiding this comment.
right that’s the question does this make a perf difference? as u have it vs master
There was a problem hiding this comment.
huh, if anything map_infer is somewhat slower in this PR than in master, and its worse with inline. so im going to revert this edit, remove the TODO comments, and in a separate branch look into if we can improve item_from_zerodim
There was a problem hiding this comment.
I can open an issue about this separately, but our current inline strategy may be doing more harm than good. GCC has a -Winline flag that will tell you when a function declared with inline isn't actually inlined, and running this on Linux appears to produce 11 MB worth of warnings with a clean build of pandas
There was a problem hiding this comment.
Yikes. Addressing the larger issue separately makes sense. For the purposes of this PR, is there something that should be changed?
There was a problem hiding this comment.
Things lgtm here as is, but let's see what Jeff thinks
There was a problem hiding this comment.
no this is fine, but yeah looking at this generally is prob a good idea.
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diff