-
-
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
Moving eval mixin methods to the eval module #979
base: master
Are you sure you want to change the base?
Conversation
While I agree that we tend to have modules and large functions and that
I do believe right now this code has a little baggage because it is improperly called from Boxing routines when instead Boxing evaluation would have a separate routine which may sometimes (and probably rarely) need to call general (complex) Evaluation. But in those conditions, I do not expect that Evaluation has to change one bit over what it would do if Boxing did not exist. So if I have this right, this is currently a flaw of the current Expression evaluation. The second complexity inside Evaluation is a fundamental or "core" aspect of all objects. For now, I don't see a problem with explicitly specifying what evaluation is for the different kinds of atoms. Except for (compound) Expression evaluation, the code is not so large. And the complexity of (compound) Expression evaluation was alluded to before. Now we come to Also, we can avoid having to call the slow general compound Expression evaluation for something like a simple arithmetic expression by calling just the parts that we need to call and skip stuff like ordering arguments, looking for rewrite rules and so on. But note, some discretion and judgement should be used here, because we can get some very ugly and complex code for very marginal gains. Notice that this customization and reusability aspect has very little use for the evaluation routine for atoms or compound Expressions. It is hard to imagine situations where one wants to call compound Expression Having said this, here might be a case for splitting core evaluation into a separate module in |
This draft starts to move part of the implementation of the
Expression.evaluation
method to themathics.eval
module. Also, the fat routinerewrite_apply_eval_step()
is decomposed into different subroutines. @rocky, I hope this helps with the upcoming work, post the 7.0.0 release.