added deprecate_nonkeyword_arguments to function where

[Jiezheng2018]

• xref Deprecate non-keyword arguments in drop #41486
• generic.where. tests added / passed
• Ensure all linting tests pass, see here for how to run them.
• whatsnew entry. -- added.

[MarcoGorelli]

Thanks @Jiezheng2018 , this is off to a good start - if you look at #41486 you'll see an example of how to add a whatsnew note and a test

[MarcoGorelli]

Getting there! 💪

[MarcoGorelli]

Thanks @Jiezheng2018 for updating!

Just left a couple of comments - please have a look at #41510 for the most up-to-date example of how to do this, as that PR got merged

[Jiezheng2018]

It has failed all Azure tests on py37_32bit. Not sure what I should do now?

[MarcoGorelli]

Thanks @Jiezheng2018, just a few more comments!

The test failure is caused by the stacklevel of another warning inside where not being set correctly - pretty sure it'll be enough to just increase the stacklevel to 3 in the lines

pandas/pandas/core/generic.py

Lines 9253 to 9258 in b3e3352

	warnings.warn(
	"try_cast keyword is deprecated and will be removed in a "
	"future version",
	FutureWarning,
	stacklevel=2,
	)

[Jiezheng2018]

Thanks a lot for the comments :) I will work on them over the weekend.

(P.S. I am really learning a lot by reading up your comments and documents and fixing error code. Really appreciated your patience with me. I might respond a bit slower in the coming week but be assured that I am commited to get this issue done :)

[MarcoGorelli]

Glad to hear you're learning a lot from this 😄

In a different PR (#41580 (comment)), Jeff has asked for other to be positional, so let's change this to be allowed_args=["self", "cond"] - sorry for having midled you!

---

EDIT: you'd already set it as positional, so my comment here was worse than useless 🤦 My bad for leaving a review when I hadn't slept that well...

[Jiezheng2018]

Glad to hear you're learning a lot from this 😄

In a different PR (#41580 (comment)), Jeff has asked for other to be positional, so let's change this to be allowed_args=["self", "cond"] - sorry for having midled you!

No problem at all. I haven't made much progress because I kept running into problems each time I do a git fetch upstream and git merge upstream/master. (My understanding is that I should do this each time before I start working, right?)

I can't merge successfully because other people are also working on the same file (which is to be expected). However, what is really confusing for me is that the merge still fails sometimes, even after I removed all my changes. Error messages are to do with uncommitted changes. Trying git commit then I start failing on other pre-commit checks, either black, flake8 or absolufy-imports. Am I doing something stupid here?

I bypassed the above eventually by discarding all of my changes and aborted the merge and used git checkout --theirs .
Is this the correct way of doing it?

[Jiezheng2018]

Hi,
I think I have managed to complete all of your requests now. Do you mind having another look? I might miss or revert some of my previous work because of my git merge issues.

[Jiezheng2018]

Morning, it seems like I failed a lot of tests again. I had to change the future warning 'stack level' back to 2, otherwise, I was failing all 'black' pre-commit tests with an error code of 123. I will do a bit more research and try to work on it later today or tomorrow.

[MarcoGorelli]

Morning, it seems like I failed a lot of tests again. I had to change the future warning 'stack level' back to 2, otherwise, I was failing all 'black' pre-commit tests with an error code of 123. I will do a bit more research and try to work on it later today or tomorrow.

Hey @Jiezheng2018 - no worries, I've added a commit to fix some things up (partially to make sure this gets done in time, and partially because I'd misled you in some previous reviews 😳 ) - do you want to have a look at the recent changes?

I had to change the future warning 'stack level' back to 2, otherwise, I was failing all 'black' pre-commit tests with an error code of 123

I'm pretty sure that's not the kind of thing the black formatter would complain about - perhaps you also changed something else and caused a syntax error by accident?

[Jiezheng2018]

Glad to hear you're learning a lot from this 😄

In a different PR (#41580 (comment)), Jeff has asked for other to be positional, so let's change this to be allowed_args=["self", "cond"] - sorry for having midled you!

EDIT: you'd already set it as positional, so my comment here was worse than useless 🤦 My bad for leaving a review when I hadn't slept that well...

No worries at all. :) Now I understand the positional keyword even better, was gonna read up on it more after work today. :) thanks a lot

[Jiezheng2018]

Morning, it seems like I failed a lot of tests again. I had to change the future warning 'stack level' back to 2, otherwise, I was failing all 'black' pre-commit tests with an error code of 123. I will do a bit more research and try to work on it later today or tomorrow.

Hey @Jiezheng2018 - no worries, I've added a commit to fix some things up (partially to make sure this gets done in time, and partially because I'd misled you in some previous reviews 😳 ) - do you want to have a look at the recent changes?

I had to change the future warning 'stack level' back to 2, otherwise, I was failing all 'black' pre-commit tests with an error code of 123

I'm pretty sure that's not the kind of thing the black formatter would complain about - perhaps you also changed something else and caused a syntax error by accident?

1. Thank you very much for the fixup. That's super helpful to see how you made the changes :)
2. I am not very sure what I did that caused the 'black' to complain so much. I did notice sometimes exactly the same code on my VS code will fail the first time and pass the second time. I might need to re-install VS code at some point or just use the terminal instead.

[MarcoGorelli]

Looks good to me, will leave open for a bit in case others have comments

[jreback]

thanks @Jiezheng2018