Misc tweaks... #1072
Misc tweaks... #1072
Conversation
rocky
force-pushed
the
misc-tweaks-in-prep-for-event-handler
branch
from
4ab5e81
to
686d375
Compare
(These were noticed in working on event tracing) `mathics/builtin/numbers/algebra.py`: black changes its autoformating `mathics/builtin/numbers/numbertheory.py`: some linting prefers triples quotes for docstrings `mathics/core/convert/sympy.py`: use accurate location for singleton class. This reduces unnecessary sympy imports `mathics/core/symbols.py`: shorten prefix and context names for SymPy variables. In tracing we will see these names a lot. A shorter prefix makes understanding expressions involving these easier to follow `CHANGES.rst`: document recent SetEnvironment addition and GetEnvironment changes `test/numbers/__init__.py`: needed if test/numbers is tested separately
rocky
force-pushed
the
misc-tweaks-in-prep-for-event-handler
branch
from
686d375
to
25f299a
Compare
mathics/core/symbols.py
Outdated
| # Prefix used for Sympy variables. | ||
| # We wan t this to be short to keep variable names short. | ||
| # In tracing values, long names make output messy. | ||
| sympy_symbol_prefix = "_m_" |
There was a problem hiding this comment.
@rocky, in the test it seems you used _mu_ as the prefix
There was a problem hiding this comment.
Good catch. I am reverting these changes for now, since I forget to update the testing. And the testing isn't catching the change either.
So all of this is more involved. I don't want to hold up other stuff because of these issues. Instead, I'll punt for now.
| Symbol("_Mathics_User_x"), | ||
| sympy.Symbol("_Mathics_User_System`_Mathics_User_x"), | ||
| Symbol("_mu_x"), | ||
| sympy.Symbol("_mu`x"), |
There was a problem hiding this comment.
I would expect that if the conversion routines haven't changed, and the prefix is _m_,
"_Mathics_User_System`_Mathics_User_x" should be now "_m_System`_m_x",
with the prefix both in the symbol name and the context.
In any case, it would be reasonable to put the prefix in the context.
There was a problem hiding this comment.
See above. Not sure why testing works when it shouldn't. Testing is probably a bit weak then.
The issue of shortening context prefixes will be addressed in a more isolated PR.
There was a problem hiding this comment.
good. Then let's merge this when you feel it is ready.
There was a problem hiding this comment.
The reason why the tests are "weak" is that we are not checking the result of the comparison. I will put a fix for this in another PR.
mathics/builtin/numbers/algebra.py: black changes its autoformatting mathics/builtin/numbers/numbertheory.py: some linting prefers triples quotes for docstrings mathics/core/convert/sympy.py: use accurate location for singleton class. This reduces unnecessary sympy imports CHANGES.rst: document recent SetEnvironment addition and GetEnvironment changes test/builtin/numbers/__init__.py: needed if testing in this directory is done separately.
(These were noticed in working on event tracing)
mathics/builtin/numbers/algebra.py: black changes its autoformattingmathics/builtin/numbers/numbertheory.py: some linting prefers triples quotes for docstringsmathics/core/convert/sympy.py: use accurate location for singleton class. This reduces unnecessary sympy importsCHANGES.rst: document recent SetEnvironment addition and GetEnvironment changestest/builtin/numbers/__init__.py: needed if testing in this directory is done separately.