Skip to content

Conversation

@bdlucas1
Copy link
Collaborator

This can happen for example if you give it a string consisting of just a comment. I'm not sure if this behavior - returning None - is correct, but at least it's consistent with what parse does.

Also I suppose there should be a test for this, but not sure where the best place for it would be.

if form is None:
form = self.form
self.last_result = expr.evaluate(self.evaluation)
self.last_result = expr.evaluate(self.evaluation) if expr else None
Copy link
Contributor

Choose a reason for hiding this comment

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

In WMA, an empty expression should result in Null (SymbolNull). I think that what we should do here instead is to check in line 147 if expr is None, and return SymbolNull.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks. I've updated the PR accordingly.

Copy link
Member

@rocky rocky Dec 19, 2025

Choose a reason for hiding this comment

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

Thanks. I've updated the PR accordingly.

Thanks. LGTM. (And thanks @mmatera for spotting.)

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@rocky rocky changed the title Don't raise excepting in evaluate if parsing expr returns None Don't raise exception in evaluate() if parsing expr returns None Dec 19, 2025
@bdlucas1 bdlucas1 merged commit 7bb5b25 into Mathics3:master Dec 19, 2025
17 checks passed
@rocky rocky deleted the fix-none-expr branch December 19, 2025 14:42
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