Skip to content

assert_*_equal issues #25135

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

Closed
AntoineD opened this issue Feb 4, 2019 · 4 comments · Fixed by #25462
Closed

assert_*_equal issues #25135

AntoineD opened this issue Feb 4, 2019 · 4 comments · Fixed by #25462

Comments

@AntoineD
Copy link

AntoineD commented Feb 4, 2019

The function assert_almost_equal may return the return value of another functions like assert_frame_equal or assert_series_equal.
But assert_frame_equal does not return any value explicitly, hence it shall always return None.
Also, assert_series_equal may or may not return a value explicitly.

Are those assertion functions usable or am I overlooking something?

@chris-b1
Copy link
Contributor

chris-b1 commented Feb 5, 2019

assert_almost_equal is not part of the public API, can you please show an example of what's wrong?

@chris-b1 chris-b1 added the Needs Info Clarification about behavior needed to assess issue label Feb 5, 2019
@AntoineD
Copy link
Author

AntoineD commented Feb 5, 2019

I was not sure about how to use assert_almost_equal, despite the name starting with assert I have looked at the code and have seen return statements, so I figured out I should check the truth of the returned value.
May be the docstring could specify that an AssertionError is raised when the assertion fails.

If that's not a public function, then please ignore this issue.

Nevertheless, I think a public function on par with the numpy allclose would be very useful (with the ability to combine absolute and relative errors).

@chris-b1
Copy link
Contributor

chris-b1 commented Feb 5, 2019

That function is not public - but it does look like the return statements are redundant so will keep this open as a code cleanup issue. Also would take that improvement to the docstring.

@chris-b1 chris-b1 added Docs Effort Low Clean good first issue and removed Needs Info Clarification about behavior needed to assess issue labels Feb 5, 2019
@chris-b1 chris-b1 added this to the Contributions Welcome milestone Feb 5, 2019
@TrigonaMinima
Copy link

TrigonaMinima commented Feb 13, 2019

@chris-b1 if I understand this correctly, the assertions part (assert_frame_equal and assert_series_equal) will remain as is, but their return values need not be returned? Only code cleanup is the removal of the return keyword?

@jreback jreback modified the milestones: Contributions Welcome, 0.25.0 Mar 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants