-
-
Notifications
You must be signed in to change notification settings - Fork 45
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
bark when a rule in Builtin.rules cannot be loaded #1006
Conversation
These are the rules that cannot be loaded and must be fixed:
|
* Improve ``mathics.core.definitions.get_tag_position`` to support more complex rules * Add a pytest to check different cases of ``get_tag_position`` * Restore ``mathics.docpipeline`` from master~3, because the current one is not properly working.
@rocky, when you have the chance, please look at this old PR. This fixes the load of some rules and prevents the include dead rules in the future. |
mathics/core/definitions.py
Outdated
"System`BlankNullSequence", | ||
) | ||
|
||
def strip_pattern_name_and_condition(pat): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add annotations:
strip_pattern_name_and_condition(pat: ExpressionPattern) -> ExpressionPattern
this will be in PR bark_if_builtin_rule_cannot_be_stored-more
This function strips it to ensure that | ||
``pat`` does not have that form. | ||
""" | ||
if pat.get_head_name() == "System`Condition": |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use SymbolCondition and SymbolPattern below. Again, this will be in a PR for your review.
We have to use get_head_name() for testing on System`Condition
because pat
can either SymbolCondition
or an <AtomPattern: System`Condition>
and in the latter case, comparing to SymbolCondition is not sufficient.
mathics/core/definitions.py
Outdated
return strip_pattern_name_and_condition(pat.elements[1]) | ||
return pat | ||
|
||
def check_is_subvalue(pattern_sv, name_sv): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add annotation for function signature:
def check_is_subvalue(pattern_sv: ExpressionPattern, name_sv: str) -> bool:
the function name check_is_subvalue()
is a poor name. First, for functions returning boolean values we use is_xxx
not check_is_xxx
. Second what is a subvalue?
In the PR that will appear I have changed the docstring to:
"""Determines if ``pattern_sv`` or any of its alternates is a pattern with name `name_sv`"""
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the PR I have changed the function name to "is_pattern_a_kind_of".
LGTM |
This is related to #1000. There are some rules included in the
Builtin.rules
property that cannot be loaded. With this PR, it this happens, a warning is printed. In any case, the idea is to fix these cases before merging this PR.