CLN: separate raising from non-raising parts of method #27151
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@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
requested changes
Jul 1, 2019
| @@ -659,7 +659,13 @@ def _try_cast_result(self, result, dtype=None): | |||
| # may need to change the dtype here | |||
| 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
| @@ -2116,25 +2126,28 @@ def _can_hold_element(self, element): | |||
| return (is_integer(element) or isinstance(element, datetime) or | |||
| isna(element)) | |||
|
|
|||
| 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
| @@ -2484,21 +2494,25 @@ def fillna(self, value, **kwargs): | |||
| value = Timedelta(value, unit='s') | |||
| return super().fillna(value, **kwargs) | |||
|
|
|||
| def _try_coerce_args(self, values, other): | |||
| def _coerce_values(self, values): | |||
| """ | |||
jreback
approved these changes
Jul 2, 2019
|
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. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.