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

Implementing py-tuple py-list py-dict py-chain #671

Merged
merged 14 commits into from
Apr 27, 2024

Conversation

astroseger
Copy link
Contributor

This PR adds py-tuple py-list py-dict py-chain into stdlibs
This PR also updates sandbox simple_import examples

@astroseger astroseger requested a review from Necr0x0Der April 18, 2024 12:14
for a in atoms:
if isinstance(a, GroundedAtom):
rez.append(groundedatom_to_python_object(a))
elif isinstance(a, ExpressionAtom):
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to add one more
elif isinstance(a, SymbolAtom):
rez.append(get_string_value(a))

or you see some problems with it?

(py-tulple (user "some message)) is not working now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in this case it is better to add it directly in groundedatom_to_python_object. @Necr0x0Der What do you think about it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Turning arbitrary atoms to Python objects is generally useful. The problem is that there can be some difference how we would like to do this. In particular, in some cases we would like to distinguish, say, S("foo") from ValueAtom("foo") after conversion to Python, while in others we wouldn't want. However, if the main intention is to have an extractor of Python grounded atoms content to Python itself, then we definitely want ValueAtom("foo") to become "foo" rather than "\"foo\"". Thus, if we want to add support for symbols within this functionality, they will be naturally mixed up, which seems ok for this purpose. OTOH, we may want to do it differently, and, say, (py-tulple (user "some message")) could result in tuple (S("user"), "some message"), so the symbol is added to the tuple, but not unwrapped from the atom. From general perspective, I'd prefer the latter behavior, but not sure if it will be convenient.
BTW, if we know that isinstance(a, SymbolAtom), then it is preferable to use a.name rather than get_string_value(a).
One more thing is that if we introduce something like groundedatom_to_python_object and add the case for Symbol there (turning it effectively to atom_to_python), shouldn't we add the case for Expression there as well? Apparently, the issue is that we may want to process expressions for tuples/lists and dicts recursively in different ways. I'd possibly start with the way of how nested lists/dicts (and their mixtures) should be represented in MeTTa (both with symbols, variables, etc.), and then consider better ways to implement this.
The question is also whether we narrowly focus on a convenient way to pass tuples and dicts to native Python functions, or we want to be able to use this in MeTTa itself as well.

@@ -212,3 +212,52 @@ def load_ascii_atom(space, name):
return {
r"load-ascii": loadAtom
}

def groundedatom_to_python_object(a):
obj = a.get_object()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if this function is needed with such implementation. You don't need to check if the type is ValueObject or OperationObject. Both of them have content field (and value or op getter). If there are no other checks, then groundedatom_to_python_object(a) is the same as obj.get_object().content. The latter is even shorter.

@astroseger
Copy link
Contributor Author

astroseger commented Apr 25, 2024

@Necr0x0Der @besSveta I've made the following changes.

  1. py-tuple py-list py-dict and py-chain now require python tuple as a single parameter. So it will work as following:
(py-tuple (1 (2 3)))    -> (1, (2,3))
(py-list (1 (2 3 ))) -> [1, [2, 3]]
(py-dict (("a" "b") ("c" "d"))) -> {"a":"b", "c" : "d"}
  1. Unit tests were added.

  2. py-dict will automatically convert Symbol into python string for keys:
    (py-dict ((a "b"))) -> {"a" : "b"}

  3. The most controversial change: all functions will pass non-grounded objects as it is (except Symbol for dictionary keys and Expressions in py-tuple and py-list).
    It means that the function (py-list (a b)) will create python list with two symbol atoms.

@Necr0x0Der Necr0x0Der merged commit 5cd8235 into trueagi-io:main Apr 27, 2024
2 checks passed
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.

4 participants