Move mathics.core.builtin initialization code into system_initialize()

[rocky]

@mmatera and @TiagoCavalcante : this is kind of important to understand.

Although we now require a frontend to explicitly call initialize_system() and this may seem more awkward, it is helpful and important for a couple of reasons.

First: it provides better modularization and reduces circular imports.

mathics.builtin is close to top-level import and, in contrast to mathics.core.system_init there are a lot of modules underneath mathics.builtin. (There are none under mathics.core.system_init).

In general, it is considered bad Python coding practice to put a lot of code in __init__.py files.

As a result, we can have more import mathics.core.system_init from a module where we can't do the same for import mathics.builtin.

Second: by placing initialization of structures which do not change outside of development we are in a better position to write an administrative tool to load all of this and then dump it as a system image file, which can be read in rather than built it up from scratch as happens now. This new function initialize_system might have a parameter to read the image file rather than do the work it does now.

In its current form though initialize_system() does as not encapsulate system loading as much as it could.

[mmatera]

@rocky, In the proposal I made in #639, there is no need to modify the front-ends. What is the improvement here over that branch?

(in that branch, there is no need to call _initialize_system_definitions() from outside of the mathics.builtin.system_init module mathics/builtin/system_init.py )

[rocky]

@rocky, In the proposal I made in #639, there is no need to modify the front-ends. What is the improvement here over that branch?

(in that branch, there is no need to call _initialize_system_definitions() from outside of the mathics.builtin.system_init module mathics/builtin/system_init.py )

There is a principle Explicit is better than Implicit.

In #639 (and master) magic initialization happens when the module is loaded. Exactly when is hard to determine, since it relies on what module imports what where and in what order.

The order of module loading for purposes of what gets initialize when is not something a programmer generally keeps track of nor wants to.

But since the intent here is to serve as a place for performing initialization that might be done to create a base image, this is something that I believe we want developers to know about and be able to know very definitely when and where this is triggered - the call to system_initialize. Again, this is something that I believe developers should be aware of.

[rocky]

BTW The main other problem I have with #639 is that tries to do too much and doesn't succeed in either activity as well as if we isolate the two concerns of reducing mathics.builtin (which does a bit of system initialization) and pulling out module loading from inside Definitions object creation.

That second aspect we have come to learn is a bit thorny and needs more attention. This PR (when done) lays a better foundation for that to occur.

[rocky]

@mmatera this is about all the time I have this week for this. I don't think I was completely successful here since I'd like to move to move out all of the code from mathics.builtin.__init__ and some still remains.

The "sanity check" which looks for that Builtins have a "summary_text" should be turned into a standalone program outside of the code, and either made a separate program or added to one of the other consistency checks.

That there are this many changes and this many commits (with squashing down a number of commits into one) is a testament to the messiness we still have in or code and the problems with adding features before cleaning up a base that is wanting.

If you have are willing and have time to move this forward more, please do. Otherwise we can merge I will iterate next week.

[mmatera]

@mmatera this is about all the time I have this week for this. I don't think I was completely successful here since I'd like to move to move out all of the code from mathics.builtin.__init__ and some still remains.

OK. I think that this is a progress.

The "sanity check" which looks for that Builtins have a "summary_text" should be turned into a standalone program outside of the code, and either made a separate program or added to one of the other consistency checks.

I think now the standalone program is there (test/consistency-and-style/test_summary_text.py) so it is safe to remove the sanity_check code.

That there are this many changes and this many commits (with squashing down a number of commits into one) is a testament to the messiness we still have in or code and the problems with adding features before cleaning up a base that is wanting.

If you have are willing and have time to move this forward more, please do. Otherwise we can merge I will iterate next week.

Since this seems a little different from what I have in mind, I will need some time to understand this. In particular, I am not convinced about the need to call the function that initializes the module from outside the module. Mainly because it is something that must be called exactly once when the module is imported. Also, before doing something that implies change the other projects, I would like to define better what is the API for this part of the code.
I will try to split this in smaller parts before the next weekend, and then let's discuss over it.

[mmatera]

What is the difference between this and

mathics-core/mathics/builtin/__init__.py

Line 50 in e1e0c44

def add_builtins(new_builtins):

?

[rocky]

What I have is a still a mess still.

I'd like to remove add_builtin from mathics/builtin/__init__.py and the call at the end to of that module. When I try though I am getting errors in evaluation. So right now duplication of code is as close as I can get.

[mmatera]

This routine is called each time Definitions(add_builtins=True) is called. And what it essentially does is to initialize (a part of) the mathics.builtins module, so if you do not call it, Definitions(add_builtins=True) can not do what it is expected. So, should not be better either
a) call it in mathics.buitlin.__init__ module.
b) import it (locally) and call it in mathics.core.definitions.Definitions.__init__, if the add_builtins==True?

[rocky]

I don't know how to resolve. I don't like a) because we want to remove stuff from mathics.builtin.__init__ - that is the whole point of this.

We want a function, initialize_system() that is explicitly called that will load will initialize all predefined Builtins.

Once that is done, we are in a position to write a routine to dump out all of this to say a pickle file and then we have the possibility to replace that code with code that reads in the dumped information instead.

Equally important, this is conceptually simpler, and coding-wise we don't have critical stuff or magic code inside the an __init__ module.