-
-
Notifications
You must be signed in to change notification settings - Fork 62
Lesser changes and possible bugs encountered adding location information #1413 #1414
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,6 @@ | |
| from mathics.core.attributes import A_NO_ATTRIBUTES | ||
| from mathics.core.convert.expression import to_mathics_list | ||
| from mathics.core.element import BaseElement, fully_qualified_symbol_name | ||
| from mathics.core.load_builtin import definition_contribute, mathics3_builtins_modules | ||
| from mathics.core.rules import BaseRule, Rule | ||
| from mathics.core.symbols import Atom, Symbol, strip_context | ||
| from mathics.core.util import canonic_filename | ||
|
|
@@ -1066,6 +1065,10 @@ def load_builtin_definitions( | |
| """ | ||
| Load definitions from Builtin classes, autoload files and extension modules. | ||
| """ | ||
| from mathics.core.load_builtin import ( | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I ran into circular dependencies from changes in #1413, and this fixed that. However, moving this down also feels right in that there are the other imports here. What this is telling us is that |
||
| definition_contribute, | ||
| mathics3_builtins_modules, | ||
| ) | ||
| from mathics.eval.files_io.files import get_file_time | ||
| from mathics.eval.pymathics import PyMathicsLoadException, load_pymathics_module | ||
| from mathics.session import autoload_files | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,7 +49,7 @@ | |
| # FIXME: should rework so we don't have to do this | ||
| # We have the character name ImaginaryI and ImaginaryJ, but we should | ||
| # have the *operator* name, "I". | ||
| special_symbols["\uF74F"] = special_symbols["\uF74E"] = "I" | ||
| special_symbols["\uf74f"] = special_symbols["\uf74e"] = "I" | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This one is weird in that black or some formatter decided to do this. Again, I'd rather switch than argue with whatever formatter decided this. |
||
|
|
||
|
|
||
| # An operator precedence value that will ensure that whatever operator | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,11 @@ | ||
| # -*- coding: utf-8 -*- | ||
|
|
||
| from typing import Any, FrozenSet, Tuple | ||
| from typing import FrozenSet, Optional, Tuple | ||
|
|
||
| from mathics_scanner.feed import LineFeeder | ||
|
|
||
| from mathics.core.definitions import Definitions | ||
| from mathics.core.element import BaseElement | ||
| from mathics.core.parser.convert import convert | ||
| from mathics.core.parser.feed import MathicsSingleLineFeeder | ||
| from mathics.core.parser.parser import Parser | ||
|
|
@@ -12,7 +14,7 @@ | |
| parser = Parser() | ||
|
|
||
|
|
||
| def parse(definitions, feeder: LineFeeder) -> Any: | ||
| def parse(definitions, feeder: LineFeeder) -> Optional[BaseElement]: | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding more precise type annotations for mypy. |
||
| """ | ||
| Parse input (from the frontend, -e, input files, ToExpression etc). | ||
| Look up symbols according to the Definitions instance supplied. | ||
|
|
@@ -22,7 +24,9 @@ def parse(definitions, feeder: LineFeeder) -> Any: | |
| return parse_returning_code(definitions, feeder)[0] | ||
|
|
||
|
|
||
| def parse_incrementally_by_line(definitions, feeder: LineFeeder) -> Any: | ||
| def parse_incrementally_by_line( | ||
| definitions: Definitions, feeder: LineFeeder | ||
| ) -> Optional[BaseElement]: | ||
| """Parse input incrementally by line. This is in contrast to parse() or | ||
| parser_returning_code(), which parse the *entire* | ||
| input which could be many line. | ||
|
|
@@ -50,19 +54,28 @@ def parse_incrementally_by_line(definitions, feeder: LineFeeder) -> Any: | |
| return convert(ast, definitions) | ||
|
|
||
|
|
||
| def parse_returning_code(definitions, feeder: LineFeeder) -> Tuple[Any, str]: | ||
| """ | ||
| Parse input (from the frontend, -e, input files, ToExpression etc). | ||
| def parse_returning_code( | ||
| definitions: Definitions, feeder: LineFeeder | ||
| ) -> Tuple[Optional[BaseElement], str]: | ||
| """Parse input (from the frontend, -e, input files, ToExpression etc). | ||
| Look up symbols according to the Definitions instance supplied. | ||
|
|
||
| Feeder must implement the feed and empty methods, see core/parser/feed.py. | ||
| ``feeder`` must implement the ``feed()`` and ``empty()`` | ||
| methods. See the mathics_scanner.feed module. | ||
|
|
||
| """ | ||
| from mathics.core.expression import Expression | ||
|
|
||
| ast = parser.parse(feeder) | ||
| source_code = parser.tokeniser.code if hasattr(parser.tokeniser, "code") else "" | ||
| if ast is not None: | ||
| return convert(ast, definitions), source_code | ||
| else: | ||
| return None, source_code | ||
|
|
||
| source_text = parser.tokeniser.source_text | ||
|
|
||
| if ast is None: | ||
| return None, source_text | ||
|
|
||
| converted = convert(ast, definitions) | ||
|
|
||
| return converted, source_text | ||
|
|
||
|
|
||
| class SystemDefinitions: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -399,6 +399,12 @@ def get_match_count(self, vars_dict: Optional[dict] = None) -> Tuple[int, int]: | |
| """The number of matches""" | ||
| return (1, 1) | ||
|
|
||
| @property | ||
| def short_name(self) -> str: | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using |
||
| return ( | ||
| self.atom.short_name if hasattr(self.atom, "short_name") else str(self.atom) | ||
| ) | ||
|
|
||
|
|
||
| # class StopGenerator_ExpressionPattern_match(StopGenerator): | ||
| # pass | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -495,6 +495,8 @@ def evaluate(self, evaluation): | |
| for rule in rules: | ||
| result = rule.apply(self, evaluation, fully=True) | ||
| if result is not None and not result.sameQ(self): | ||
| if result.is_literal: | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is probably just an oversight on our part. Stuff like this becomes apparent when you start looking at traces of expression evaluations. |
||
| return result | ||
| return result.evaluate(evaluation) | ||
| return self | ||
|
|
||
|
|
@@ -743,7 +745,12 @@ class BooleanType(SymbolConstant): | |
| the constant is either SymbolTrue or SymbolFalse. | ||
| """ | ||
|
|
||
| pass | ||
| @property | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thiink @mmatera had mentioned that redfining True and False in MMA, if it is possible, will horribly break MMA. I recall we had a discussion about whether the parent class SymbolConstant could be a literal. Right now it is not. However True and False I have a harder time seeing why it wouldn't be literal.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, |
||
| def is_literal(self) -> bool: | ||
| """ | ||
| We don't allow changing Boolean values True and False. | ||
| """ | ||
| return True | ||
|
|
||
|
|
||
| def symbol_set(*symbols: Symbol) -> FrozenSet[Symbol]: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,8 +26,18 @@ def skip_trivial_evaluation(expr, status: str, orig_expr=None) -> bool: | |
| * the evaluation is a literal that evaluates to the same thing, | ||
| * evaluating a Symbol which the Symbol. | ||
| * Showing the return value of a ListExpression literal | ||
| * Evaluating Pattern[] which define a pattern. | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This change is inspired by looking at TraceEvaluation. There is no benefit in watching an evaluation of |
||
| """ | ||
| from mathics.core.expression import Expression | ||
| from mathics.core.symbols import Symbol, SymbolConstant | ||
| from mathics.core.systemsymbols import SymbolBlank, SymbolPattern | ||
|
|
||
| if isinstance(expr, tuple): | ||
| expr = expr[0] | ||
|
|
||
| if isinstance(expr, Expression): | ||
| if expr.head in (SymbolPattern, SymbolBlank): | ||
| return True | ||
|
|
||
| if status == "Returning": | ||
| if ( | ||
|
|
@@ -39,7 +49,9 @@ def skip_trivial_evaluation(expr, status: str, orig_expr=None) -> bool: | |
| return True | ||
| pass | ||
| if isinstance(expr, Symbol) and not isinstance(expr, SymbolConstant): | ||
| # Evaluation of a symbol, like Plus isn't that interesting | ||
| # Evaluation of a symbol, like Plus isn't that interesting. | ||
| # Right now, SymbolConstant are not literals. If this | ||
| # changes, we don't need this clause. | ||
| return True | ||
|
|
||
| else: | ||
|
|
||
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.
Change to BaseElement reduces mypy complaints. It may very well be that Expression is more correct, but I don't know how to get mypy to agree with this. So here I switched rather than argue with mypy.