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

Make Nulls Terminal #679

Merged
merged 1 commit into from
Mar 8, 2024
Merged

Conversation

hudson-ai
Copy link
Collaborator

Teeny-tiny little PR: make Null inherit from Terminal.

As far as I can tell, this is only really useful for preventing something rather silly from blowing up:

@guidance
def null_grammar(lm):
  return lm

Why would you write this? Don't ask.

Currently, calling this will raise an AttributeError: 'Null' object has no attribute 'values' when replace_grammar_node does its thing.

An alternative fix would be to explicitly add Null to this isinstance check in replace_grammar_node:

def replace_grammar_node(grammar, target, replacement):
  ...
    # We are done with this node if it's a terminal
    if isinstance(current, (Terminal, ModelVariable)):
            continue

But I think that this PR is a bit more elegant (unless anyone can think of a reason that Null wouldn't be Terminal).

All tests passing on my end :)

@slundberg
Copy link
Collaborator

That's a corner case I had not run across! Thanks @hudson-ai looks good to me.

@slundberg slundberg merged commit 3f1e925 into guidance-ai:main Mar 8, 2024
5 checks passed
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.

2 participants