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

Move more code out of mathics.builtin.__init__ #871

Merged
merged 2 commits into from
Jul 1, 2023

Conversation

rocky
Copy link
Member

@rocky rocky commented Jun 25, 2023

Moved to mathics.eval.builtin. Possibly not the best place, but it is better and is a start.

@rocky rocky requested a review from mmatera June 25, 2023 17:06
@mmatera
Copy link
Contributor

mmatera commented Jul 1, 2023

LGTM.

My only comment is that I think it mathics.core.builtins would be a better place than mathics.eval.builtin for the code used to load builtins.

My guess is that at some point mathics.core.builtin namespace should contain only subclasses of Builtin that define built in symbols, and the loading mechanisms should be in the "core" of the library. But this is something we can discuss later: the split proposed here seems a good starting point.

@rocky
Copy link
Member Author

rocky commented Jul 1, 2023

LGTM.

My only comment is that I think it mathics.core.builtins would be a better place than mathics.eval.builtin for the code used to load builtins.

My guess is that at some point mathics.core.builtin namespace should contain only subclasses of Builtin that define built in symbols, and the loading mechanisms should be in the "core" of the library. But this is something we can discuss later: the split proposed here seems a good starting point.

I had been thinking about this too, and I think mathics.core is better.

The bigger picture here is that I'd like to be able speed up initial startup time by loading only a minimal set of built in modules, and have the others load as they get called. In GNU Emacs this is called autoloading.

Moved to mathics.eval.builtin. Possibly not the best place, but it is
better and is a start.
@rocky rocky force-pushed the builtin-init-code-reduce branch from 62594a1 to 162f729 Compare July 1, 2023 20:20
@rocky rocky force-pushed the builtin-init-code-reduce branch from f7f3473 to fb0889a Compare July 1, 2023 21:46
@mmatera
Copy link
Contributor

mmatera commented Jul 1, 2023

@rocky, thanks. Merge this when you feel it is ready.

@rocky rocky merged commit ad77ccf into master Jul 1, 2023
@rocky rocky deleted the builtin-init-code-reduce branch July 1, 2023 23:48
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