removing n_evaluation

[mmatera]

This finished the changes from expr.to_python(n_evaluation=evaluation) to eval_N(expr, evaluation).to_python()

[TiagoCavalcante]

LGTM

[rocky]

Line 1386 or thereabouts has an assert that n_evaluation is None. So this isn't needed. n_evaluation, a relatively new feature, is a hack that should not have been added in the first place.

[mmatera]

Indeed, this line should be removed. Regarding n_evaluation, it is a feature that I proposed to add before we have the eval_N function, and I think that was a useful intermediate step. In any case, if we still want that Expression(SymbolFunction,...).to_python(n_evaluation=evaluation) still produce a callable Python object, we can not remove the parameter completely.

[mmatera]

BTW, if we want that to_python always returns a "native" Python object, one possibility for Expressions that do not have a clear Python equivalent would be to return a callable object, like

    def result_callable():
        res = expr.evaluate(evaluation)
        if res.sameQ(expr):
             return expr
        else:
             expr.to_python()
    return result_callable

Then, if at some point the expression becomes evaluable and produces a Python native object, then the result would be a Python object...

[rocky]

Regarding n_evaluation, it is a feature that I proposed to add before we have the eval_N function, and I think that was a useful intermediate step.

I'd like to be very clear about this. In my view this was an intermediary step because there was myopic thinking about the code an a lack of a bigger picture about how interpreters normally work.

The entire operation represented as a class is the kind of thing you find in toy interpreters but not performant interpreters. Performant interpreters do not use class methods to implement built-in operations.

For a list of Python built-in operations see https://docs.python.org/3/library/dis.html#opcode-collections . For a list of GNU Emacs operations see https://github.com/rocky/elisp-bytecode . You can find the same thing for Ruby, Java, Javascript, and so on. It is a necessary prerequisite say if you want to have add JIT technology for example (which we are a very long way from being able to contemplate).

I mention to be aware of this so as not do this as much in the future. There has been a rush to just plunge into into new areas where much is not known either in terms of the existing code or of conventional practice. And that sometimes leads to an equal effort to remove it later.

Remind me why would we want to "support" Expression(SymbolFunction,...).to_python(n_evaluation=evaluation)?

[rocky]

BTW, if we want that to_python always returns a "native" Python object, one possibility for Expressions that do not have a clear Python equivalent would be to return a callable object, like

    def result_callable():
        res = expr.evaluate(evaluation)
        if res.sameQ(expr):
             return expr
        else:
             expr.to_python()
    return result_callable

Then, if at some point the expression becomes evaluable and produces a Python native object, then the result would be a Python object...

This is a possibility but there would be a different method called. As I have said in the past there was this pervasive vaguess which assists a muddling of concepts. to_python() is used for different kinds of situations which haven't been clearly specified. And that is why we have weird or hybrid behavior such as returning a string when conversion fails instead of raising an exception. (And letting some other routine convert this to a string then if that is what it needs to do).

As I have alluded to before, to_python_literal() would be a better name for at least one of the disentangled functions.

[mmatera]

Ok - point taken. I think you are correct. That PR was way too big to easily grasp all that is going on. Especially the relevant part here where this interface was changed. (Github says the diff is too large to display)

OK. That was because I was much less aware than now about how to make a readable PR. This is something that I learn in the hard way during this time...

I should say that I now understand a lot about how this code needs to change to get it to the point where it seems like a normal non-toy interpreter. And I think to some degree we all understand some of this.

It is going to be a long haul. I'd ask then to please stay away from the parser and scanner and compile methods for now. Pattern matching needs also going over and that is on the list.

The way symbols are bound to variables is another thing that is going to be a mess.

Evaluation we've really only started to go over. It is nowhere near gone over though.

Yes, I agree with all that. But then, we can take two positions: leave the code as it is, diagram a master plan, then rework all the existing pieces, and then (let's say, in a couple of years) see if we can start improving the functionality.

The other way (the one in which I think I could be useful now) is to fix peripherical parts with (maybe suboptimal) code, and to add tests to reinforce the compatibility with WMA. This is what I was trying to do during the last months.

[rocky]

Yes, I agree with all that. But then, we can take two positions: leave the code as it is, diagram a master plan,

There has been a rough plan. I was making my way over the rewrite-apply-eval process. I had gotten to the top-level evaluation. That was blocking Formatting and Boxing work you wanted to do. So I stopped that process so that you would have free reign over Form, Boxing, and Format. While you are doing that I am looking to expand the use of ListExpression and customized kinds of expressions. Association is a natural for this as well.

The Boxing and Formatting work quickly spilled into scanning and parsing - rightly or wrongly. I'll handle issues that spill over there.

The other way (the one in which I think I could be useful now) is to fix peripherical parts with (maybe suboptimal) code, and to add tests to reinforce the compatibility with WMA. This is what I was trying to do during the last months.

In the places where we have no choice, okay. But in the areas where I have a clearer idea of what needs to be done, let's wait on that. With respect specifically to Forms, Boxing, and Format, this is a clear area where you know more about this than I can even hope to understand, so focus on that. The plan that you have put together seems good so far. So just continue with that.

As for this PR, let's put back that assert and remove the more complex code that deprecating something that we now know to be ill-defined.

[mmatera]

OK. The assert was removed, and I left the code for converting Expression(SymbolFunction, ...) as it was before, except by the name of the parameter (I have removed the "n_"). If a call is done with the "n_evaluation" parameter, a deprecation warning is shown, and then continue almost as it was before, except that instead of using the Expression(SymbolN,...).evaluate(n_evaluation) idiom, a eval_N() call is used.

[rocky]

Please no deprecation warning for something that should not have been added.

This PR I thought (and actually the previous one) were supposed to remove uses, not deprecate them. The "assert" checks that. In a little while the assert can be removed as well. Thanks.

In a little while the assert could be removed as well.

[mmatera]

I commented out both the assert and the code that shows the deprecation. I am going to leave this as a draft, and eventually use this discussion (and the changes) when we want to go over this method.

[mmatera]

Also, I fixed a typo here, which was avoiding that this module gets loaded.

[mmatera]

Fixed also in #560.

[TiagoCavalcante]

@mmatera what is the status of this PR?