-
-
Notifications
You must be signed in to change notification settings - Fork 18.6k
CLN: remove merge-pr script, clean Exception in tm, doc #28442
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
Conversation
title = title.encode("raw_unicode_escape") | ||
except Exception: | ||
pass | ||
|
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.
@pandas-dev/pandas-core : Out of curiosity, how many people actually use this nowadays?
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.
Never used it, happy to get rid of it if nobody does.
I never use it anymore, and I think we fully switched to using github's button to merge PRs. But maybe some people still use it to fetch PR's branchs and push back there? |
yeah i think we can blow away the merge script |
Updated to remove merge-pr.py |
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.
Looks good - minor comments
@@ -50,6 +50,7 @@ def test_dask(df): | |||
assert ddf.compute() is not None | |||
|
|||
|
|||
@pytest.mark.filterwarnings("ignore:Panel class is removed") |
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.
Was this not causing CI failures before? Thought we failed on warnings like this
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.
this is showing up locally
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.
This should not be needed with latest xarray I suppose
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.
lgtm
Thanks! |
I count 131 of these ATM, some of which we'll end up marking as intentionally non-specific.