Skip to content

Conversation

gohil-jay
Copy link
Collaborator

image

@github-actions github-actions bot added the needs changelog Might need a changelog entry label Jul 1, 2022
if (list(self.axes)==list(hist2.axes)):
return ""
else:
return "The axes [" + "\033[91m" + str(list(self.axes)) + " and " + str(list(hist2.axes)) + "\033[0m" + "] are not equal."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return "The axes [" + "\033[91m" + str(list(self.axes)) + " and " + str(list(hist2.axes)) + "\033[0m" + "] are not equal."
return f"The axes [\033[91m{list(self.axes)} and {list(hist2.axes)}\033[0m] are not equal."

F-strings were made for this. :) Also I'd pull out the color formatting into a variable, like red and reset. Actually, there are lots of concerns with color formatting (needs to be disablable and such), at least rich can auto-format this, so probably leave the color formatting off for the moment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I bet if you do import rich; rich.print(h.compare(h2)) this would still be colorful. ;)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I was going to do something similar (with termcolor), but then though I'd do it without importing a library..

@gohil-jay
Copy link
Collaborator Author

@henryiii, pre-commit throws few errors... Any feedback/suggestion?

@henryiii
Copy link
Member

henryiii commented Jul 3, 2022

It's complaining because you are using the result of re.search without checking to see if it has a result first. I think the best fix, though, is to use ._storage_type, which contains the storage type of the histogram, rather than trying to parse it out of the repr.

@gohil-jay
Copy link
Collaborator Author

I've been looking for something like _storage_type since the beginning (couldn't find it on docs or book here)! But alright now it should work :)

gohil-jay added 3 commits July 3, 2022 16:56
Redefined storage check & removed variance check (as variances is based on values that are already checked)
@gohil-jay
Copy link
Collaborator Author

Seems alright now (?)

@henryiii henryiii marked this pull request as draft July 5, 2022 13:11
@henryiii
Copy link
Member

Note: the goal is to support pytest_assertrepr_compare https://docs.pytest.org/en/latest/how-to/assert.html#defining-your-own-explanation-for-failed-assertions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs changelog Might need a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants