CLN: clean up blocks.py #36534
CLN: clean up blocks.py #36534
Conversation
|
Reverted some changes of unused arguments in function, they still fail tests. |
|
can you merge master; unrelated test failures should be fixed |
|
done @jbrockmendel |
| try_cast: bool, default False | ||
| axis : int, default 0 |
There was a problem hiding this comment.
What this parameter try_cast is for? Looks like it is not used in the function. It will be better for the code cleanup if you delete it from the function signature and everywhere it gets called.
There was a problem hiding this comment.
Sure, I saw quite some of these arguments in other functions as well (ones which are not used), but when I remove them, some tests fail. So would it be safe to remove those in the tests or the tests themselves as well?
There was a problem hiding this comment.
It is hard to say why tests fail in this case without looking at the traceback.
Need to check whether the test itself calls the method with all the arguments, or the method is called somewhere else. In the latter case one needs to change all calls of the method (that could probably be called in multiple places).
There was a problem hiding this comment.
if its just that the tests pass the unused arg directly, then its fine to remove the arg from the test too.
If you've got some easy things (like this PR so far) and some less-obvious things, usually best to keep them as separate PRs.
|
thanks @erfannariman |
Clean up while going through code. Will probably check more files.