CLN: separate raising from non-raising parts of method #27151
Conversation
|
@jorisvandenbossche CI fails look unrelated, ami missing something? |
|
@mroeschke mentioned those should be fixed by a commit in #27144 (comment) |
gfyoung
added
Clean
Internals
|
yep, that fixed it, thanks. |
jreback
left a comment
jreback
left a comment
There was a problem hiding this comment.
lgtm. if you can fix the doc-strings (on the super class); ok to remove from the other _coerce_values.
| return maybe_downcast_to_dtype(result, dtype) | ||
|
|
||
| def _try_coerce_args(self, values, other): | ||
| def _coerce_values(self, values): |
There was a problem hiding this comment.
if possible can you add types (ok for future PR) and full doc-strings
pandas/core/internals/blocks.py
Outdated
|
|
||
| def _try_coerce_args(self, values, other): | ||
| def _coerce_values(self, values): | ||
| """ |
There was a problem hiding this comment.
you can omit the doc-strings (as will inherit from super class)
pandas/core/internals/blocks.py
Outdated
|
|
||
| def _try_coerce_args(self, values, other): | ||
| def _coerce_values(self, values): | ||
| """ |
|
lgtm. ping on green. would really love a typing PR on blocks :-> |
|
psuedo-ping. CI has been in this state for 13 hours |
|
CI finished, seems like Github missed the notification. FYI @jbrockmendel let me know if you want to be added to the azure team so you can restart builds. Just need an azure account name. |
5 participants
There is lots of code in core.internals inside try/except blocks, in particular calls to _try_coerce_args. It is tough to reason about these because it is often unclear what the raising cases are. This simplifies that problem by separating
values, args = self._try_coerce_args(values, args)intovalues = self._coerce_values(values)andargs = self._try_coerce_args(args)(Note the former doesnt have a "try" in the name because it never raises).Also: removed a never-used
ndimkwarg fromBlock.make_blockand removed a now-redundantDatetimeTZBlock.copymethod.