CLN: simplify maybe_convert_objects, soft_convert_objects #27444
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
… unreachable branches
jbrockmendel
changed the title
jreback
requested changes
Jul 20, 2019
pandas/core/dtypes/cast.py
Outdated
|
|
||
| else: | ||
| values = lib.maybe_convert_objects(values, convert_datetime=convert_dates) | ||
| if values.dtype == np.object_: |
pandas/core/dtypes/cast.py
Outdated
| values = lib.maybe_convert_objects( | ||
| values, convert_timedelta=convert_timedeltas | ||
| ) | ||
| if values.dtype == np.object_: |
| @@ -2780,44 +2784,39 @@ def is_bool(self): | |||
| return lib.is_bool_array(self.values.ravel()) | |||
|
|
|||
| # TODO: Refactor when convert_objects is removed since there will be 1 path | |||
| def convert(self, *args, **kwargs): | |||
| def convert( | |||
There was a problem hiding this comment.
side issue is that a lot of this can be cleaned up now that convert_objects is gone
There was a problem hiding this comment.
I think that deprecation was before I got here, so it isn't clear to me what else can be cleaned up
There was a problem hiding this comment.
maybe just remove the comment, you have cleaned up reasonably. you can try looking in history but if not ok.
jreback
added
Clean
Internals
jreback
reviewed
Jul 20, 2019
| numeric: bool = True, | ||
| timedelta: bool = True, | ||
| coerce: bool = False, | ||
| ): | ||
| """ attempt to coerce any object types to better types return a copy of |
There was a problem hiding this comment.
add typing to the returns would be good here (and maybe Paramters / Returns to the doc-string), but can be in the future. I think adding doc-strings on routines that we are most likely going to keep is worth the effort (so use your judgement on this).
|
ping |
|
thanks |
quintusdias
pushed a commit
to quintusdias/pandas_dev
that referenced
this pull request
Aug 16, 2019
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.
Block.replace has code for dispatching to either maybe_convert_objects or soft_convert_objects, but the former is only reached from the tests. So rip that out and use soft_convert_objects.
Both of the functions have branches that are unreachable, so rip those out, along with adding types and validation.