Skip to content
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

BUG: Replaced ValueError exception with TypeError exception in df.where() function #56358

Closed
wants to merge 13 commits into from

Conversation

DarthKitten2130
Copy link
Contributor

Replaced ValueError exception with TypeError as it is more appropriate for the error it serves

  • [] closes issue BUG: df.mask with int df throws ValueError instead of TypeError #56330 where there was a consensus that a TypeError is more appropriate for the following error: "Boolean array expected for the condition, not (non-bool type)". Since ValueErrors are raised when the type is correct but the value is incorrect, whereas in this function we are raising an exception because of a wrong type, hence the TypeError.

DarthKitten2130 and others added 6 commits August 20, 2023 19:32
Added an example for standard functions for the map function
Added documentation for extra keyword arguments in the map method
Used round function instead of defining a function in the example
…nction

Replaced ValueError exception with TypeError as it is more appropriate for the error it serves
@DarthKitten2130 DarthKitten2130 changed the title Replaced ValueError exception with TypeError exception in df.where() function BUG: Replaced ValueError exception with TypeError exception in df.where() function Dec 6, 2023
@DarthKitten2130 DarthKitten2130 marked this pull request as draft December 6, 2023 11:28
@DarthKitten2130 DarthKitten2130 marked this pull request as ready for review December 6, 2023 11:29
@rhshadrach
Copy link
Member

Thanks for the PR! Looks like the tests need adjusted too.

@rhshadrach rhshadrach added Bug Error Reporting Incorrect or improved errors from pandas labels Dec 7, 2023
@DarthKitten2130
Copy link
Contributor Author

Yeah I'm super confused because I literally only changed two keywords in the entire file and there was no way it messed up so much that 27 tests failed. Will I have to re-Pr this or will you guys fix the test stuff? Or do I need to change anything in my PR?

@rhshadrach
Copy link
Member

We test that the correct error message is raised, and you're changing that behavior. Here is an example of a failing test:

https://github.com/pandas-dev/pandas/actions/runs/7110780716/job/19357783827?pr=56358#step:8:1536

with pytest.raises(ValueError, match=msg):
    df.where(cond)

Your change now makes it so that a TypeError is raised, and so this test is now failing. The test needs to be updated to check for a TypeError.

@DarthKitten2130
Copy link
Contributor Author

ohh that actually makes a lot of sense. this is my first time contributing to a codebase (did some documentation work in the past) so I had no idea about the testing methods. Do I have to modify those tests in this PR or is that something that the moderators take care of?

@rhshadrach
Copy link
Member

Do I have to modify those tests in this PR or is that something that the moderators take care of?

The tests need to be modified so that they're passing in this PR.

@tanvincible
Copy link

@DarthKitten2130 Are you still working on it?

@DarthKitten2130
Copy link
Contributor Author

DarthKitten2130 commented Dec 14, 2023

Yeah actually I'm in the process of modifying the tests. Please do not close this PR I just haven't been finding the time.

@DarthKitten2130 DarthKitten2130 marked this pull request as draft December 14, 2023 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Error Reporting Incorrect or improved errors from pandas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants