Skip to content

[Testing] Allow Capitalized name in CompareBeforeAfter #15568

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.

Sign up for GitHub

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

Merged
Lunderberg merged 3 commits into from
Aug 17, 2023
Merged

[Testing] Allow Capitalized name in CompareBeforeAfter #15568

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a3294f7
[Testing] Allow Capitalized name in CompareBeforeAfter
vinx13 Aug 15, 2023
16a476e
Address comments
vinx13 Aug 16, 2023
b8fa0d3
Address comments
vinx13 Aug 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Address comments
  • Loading branch information
@vinx13
vinx13 committed Aug 16, 2023
commit 16a476e597a2de5cc65b60612868e4374e0a89da
4 changes: 4 additions & 0 deletions python/tvm/testing/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1953,6 +1953,10 @@ def expected(A: T.Buffer(1, "int32")):
"""

def __init_subclass__(cls):
assert (hasattr(cls, "before") and hasattr(cls, "Expected")) or (
hasattr(cls, "Before") and hasattr(cls, "Expected")
), "The subclass of CompareBeforeAfter should have either 'before' and 'expected', or "
"'Before' and 'Expected' defined."
for name in ["before", "Before"]:
Copy link
Copy Markdown
Contributor

@Lunderberg Lunderberg Aug 16, 2023

Choose a reason for hiding this comment

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

Can we add an assert that at most one of the two options are present?

Copy link
Copy Markdown
Contributor

@Lunderberg Lunderberg Aug 16, 2023

Choose a reason for hiding this comment

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

I'm not sure if should be that strong of an assert, as that would prevent defining an intermediate test that defines transform and expected, but doesn't define before. This could be useful for defining several tests that should all normalize to produce the same result. This would also break existing cases where the transform is defined, then used across several tests in a file.

What if instead, we were to assert that no duplicates names exist?

assert len([getattr(cls,name) for name in ['before','Before'] if hasattr(cls,name)]) <= 1
assert len([getattr(cls,name) for name in ['expected','Expected'] if hasattr(cls,name)]) <= 1

if hasattr(cls, name):
cls.before = cls._normalize_before(getattr(cls, name))
Expand Down