Underscore instead of grave tilde in sympy names as context separators #1536
Conversation
mmatera
changed the base branch from
master
to
good_bye_sympy_dummy_attribute
mathics/core/convert/sympy.py
Outdated
| return Expression(SymbolC, Integer(int(name[1:]))) | ||
| if name.startswith(SYMPY_SYMBOL_PREFIX): | ||
| name = name[len(SYMPY_SYMBOL_PREFIX) :] | ||
| name = name[len(SYMPY_SYMBOL_PREFIX) :].replace("_", "`") |
There was a problem hiding this comment.
This idiom appears about 3 times in this PR. Maybe it should be a little function?
| def sympy_name(mathics_symbol: Symbol): | ||
| """Convert a mathics symbol name into a sympy symbol name""" | ||
| return SYMPY_SYMBOL_PREFIX + mathics_symbol.name | ||
| return SYMPY_SYMBOL_PREFIX + mathics_symbol.name.replace("`", "_") |
There was a problem hiding this comment.
SYMPY_SYMBOL_PREFIX + mathics.symbol.name.replace("``'", "_") seems like an idiom. Maybe it could be turned into a function?
There was a problem hiding this comment.
In this case, is already a function. What is not a function yet is the conversion in the opposite direction.
|
|
||
| def testSymbol(self): | ||
| self.compare(Symbol("Global`x"), sympy.Symbol(f"{SYMPY_SYMBOL_PREFIX}Global`x")) | ||
| self.compare(Symbol("Global`x"), sympy.Symbol(f"{SYMPY_SYMBOL_PREFIX}Global_x")) |
There was a problem hiding this comment.
This too, feels like an idiom that is used a couple of times. Maybe turn into a function, too.
The thing about making functions like this that I appreciate is that the function name signals something, and a docstring is available to explain what's going on better.
|
LGTM. Please consider DRYing the code. |
rocky
left a comment
rocky
left a comment
There was a problem hiding this comment.
Thanks for the additional changes.
mmatera
force-pushed
the
underline_instead_of_grave_tilde_in_sympy_names
branch
from
4cd7284 to
ba2c6bf
Compare
…of_grave_tilde_in_sympy_names
This continues #1535, by changing the way in which symbol names are translated to sympy.