Skip to content

Additive update to retain the "is_valid" property that the safety 1.2… - #1627

Open
bollacker wants to merge 1 commit into
mainfrom
kurt_arbiter_fix
Open

Additive update to retain the "is_valid" property that the safety 1.2…#1627
bollacker wants to merge 1 commit into
mainfrom
kurt_arbiter_fix

Conversation

@bollacker

Copy link
Copy Markdown
Collaborator

… DAG generates

@bollacker
bollacker requested a review from a team as a code owner August 27, 2026 00:32
@bollacker
bollacker deployed to Scheduled Testing August 27, 2026 00:32 — with GitHub Actions Active
@github-actions

Copy link
Copy Markdown

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

def is_valid(self) -> bool:
"""Whether the verdict is usable for scoring.

Always true for a plain Safety. Subclasses whose verdict has a failure

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the child classes all have this method/property, presumably declaring it here is to force any future child class to have it too, right? If that's so, I'd mark it abstract and/or raise NotImplementedError, instead of always returning True, because that could lead to sneaky bugs.

@bkorycki

bkorycki commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

I think this was already solved. The change is merged into Roger's working branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants