Replies: 3 comments 2 replies
-
In my opinion, we can get rid of this. |
Beta Was this translation helpful? Give feedback.
2 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
In working on #635 there is this curious
builtin_filename
parameter added toDefinitions.__init__()
that has been around since the beginning and has, as far as I can tell, never been used.I think the intent was to speed up system builtin loading by pickling the pre-defined builtin definitions.
If someone is interested, we could time the difference between running the "contribute" function over all the builtin functions, versus doing a pickle'd load into the global system_builtins. (Hmm. maybe I should change the name to predefined builtins?)
(There are a lot of bad names used in the code. "contribute" is a fancy word for "add" and just as vague. What is being contributed? So "add_definitions" or "add_whatever" would be a simpler, clearer name. Other examples of cutsy, or fancy sounding but dumb and unhelpful names are "numerify" and Instanceable".)
However if this is deemed a reasonable win, adding a parameter to
Definitions.__init__()
is not the way to do this. Well, in a world were predefined builtins are constantly checked to see if they need to be loaded, maybe this is more tenable. But in the normal world where you initialize system stuff once, a cleaner way is to write a standalone program to create the pickle file and add a flag on startup that the user has to specify that just reads the pickle file. No timestamp cleverness on module files is needed. Personally, I think all of this just clutters the code for a kind of thing that is best just left as a flag.And even if you do want to compare timestamps, this can be done outside of the program using
make
, and it can kick off the standalone program to recreate the pickle file.Beta Was this translation helpful? Give feedback.
All reactions