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.
CLN: simplify maybe_convert_objects, soft_convert_objects #27444
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CLN: simplify maybe_convert_objects, soft_convert_objects #27444
Changes from all commits
bc1c21a
bb7cb11
6f28f0b
c0be483
dc3d1a1
aafb5f3
37942b1
ebc8b33
b85662f
7137e98
9636301
b17489a
1ad3f76
2e3eb9d
b32f652
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe just remove the comment, you have cleaned up reasonably. you can try looking in history but if not ok.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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).