Skip to content
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

Load single builtin module #903

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

rocky
Copy link
Member

@rocky rocky commented Aug 13, 2023

Inch forward towards being able to reduce startup time by loading only essential module.

The change here is that we can now import a single module by import name, e.g. mathics.builtin.quantum_mechanics.

Note that frontends may need to reload sessions, which is not desirable.

@rocky rocky requested a review from mmatera August 13, 2023 23:27
@rocky rocky marked this pull request as draft August 14, 2023 11:59

# First, load in many modules except quantum_mechanics.
_builtins = {}
import_and_load_builtins(exclude_files={"quantum_mechanics"}, clean_all=True)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pytest loads all the tests before running them. So, as other unit tests load test.helper, and it calls import_and_load_builtins, when we reach this test, all the modules are already loaded. So, if we want to do a test over a "fresh" instance of the module, we need this second parameter.

@mmatera
Copy link
Contributor

mmatera commented Aug 19, 2023

In the back of my mind, I have the feeling that it does not make too much sense to store these lists and dictionaries outside of the Definitions object. In any case, I guess this PR is a good step forward.

@rocky
Copy link
Member Author

rocky commented Aug 20, 2023

In the back of my mind, I have the feeling that it does not make too much sense to store these lists and dictionaries
outside of the Definitions object. In any case, I guess this PR is a good step forward.

I agree that some of these should not exist and are signs of things that are still misfeatures in the code. Some information is used by the document system. The builtin operator precedence tables may be legitimate somehow, but not in this form. I don't mind having tables that exist that indicate various correspondences between SymPy and Mathics3. It might be somewhere else though.

It has taken something like 3 or 4 iterations just to get to this stage, which is annoying and has been maddening and reflects how ill-designed things have been, make worse by "incremental" changes. It reinforces why going forward we should be taking a more high-level view inspired more by existing patterns for interpreters and Python coding.

@rocky rocky marked this pull request as ready for review August 20, 2023 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants