Conversation
to comments and names.
mmatera
left a comment
There was a problem hiding this comment.
@rocky I took note of your comments and I have incorporated some of them to #1140. If you agree, we can close this PR.
What remains open, and I think that would deserve some extra discussion, is the role of InstanceableBuiltin / CustomizableBuiltin and AtomBuiltin in the Mathics' class hierarchy. Both classes refer to objects that both define an interface and also hold "live data", so represent both an Expression and a Python object that contain non-trivial data structures associated with that expression, that we shouldn't duplicate.
|
|
||
|
|
||
| class InstancableBuiltin(Builtin): | ||
| class CustomizableBuiltin(Builtin): |
There was a problem hiding this comment.
Actually, I wouldn't change this name: InstancableBuiltin (InstanceableBuiltin) is a class representing Builtin objects that make sense to instantiate in runtime, storing different data on each instance. For example, PatternObject are InstanceableBuiltin.
Builtinobjects, on the other hand, are just instantiated once when a module is loaded, and all the data they store belongs to the class. Actually, if you do not specify the opposite, each time you instantiate a Builtin, what you actually create is an Expression.
I think that this way to work is not very intuitive, and probably could be implemented in a simpler way, but this would require a complete re-factory of all the kernel.
|
Sure - close the PR is the useful ideas have been incorporated and there is
no benefit of having this any more.
As for the name, let me think of something. The name Instanceable feels
really awkward. I am sure there must be something more appropriate and less
awkward.
And as you say there might be a totally different organization for this.
But I don't have time to think about it right now.
…On Thu, Mar 4, 2021 at 8:39 AM Juan Mauricio Matera < ***@***.***> wrote:
***@***.**** commented on this pull request.
@rocky <https://github.com/rocky> I took note of your comments and I have
incorporated some of them to #1140
<#1140>. If you agree, we can
close this PR.
What remains open, and I think that would deserve some extra discussion,
is the role of InstanceableBuiltin / CustomizableBuiltin and AtomBuiltin
in the Mathics' class hierarchy. Both classes refer to objects that both
define an interface and also hold "live data", so represent both an
Expression and a Python object that contain non-trivial data structures
associated with that expression, that we shouldn't duplicate.
------------------------------
In mathics/builtin/base.py
<#1162 (comment)>:
> @@ -366,7 +365,14 @@ def get_option_string(self, *params):
return None, s
-class InstancableBuiltin(Builtin):
+class CustomizableBuiltin(Builtin):
Actually, I wouldn't change this name: InstancableBuiltin (
InstanceableBuiltin) is a class representing Builtin objects that make
sense to instantiate in runtime, storing different data on each instance.
For example, PatternObject are InstanceableBuiltin.
Builtinobjects, on the other hand, are just instantiated once when a
module is loaded, and all the data they store belongs to the class.
Actually, if you do not specify the opposite, each time you instantiate a
Builtin, what you actually create is an Expression.
I think that this way to work is not very intuitive, and probably could be
implemented in a simpler way, but this would require a complete re-factory
of all the kernel.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1162 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAACFE2W4XNB2X7RNAVJE4TTB6ESHANCNFSM4X6ZPKHA>
.
|
Back to you @mmatera for some small changes.