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

println! on a string obtained from py-dot does not work as it should #841

Open
ngeiswei opened this issue Jan 30, 2025 · 7 comments
Open
Labels
bug Something isn't working python

Comments

@ngeiswei
Copy link
Contributor

ngeiswei commented Jan 30, 2025

Describe the bug
Calling println! on a string resulting from a py-dot outputs the string itself instead of printing it.

To Reproduce
Steps to reproduce the behavior:

  1. Compile last revision of MeTTa (6dd85ed as of this writing)
  2. Run the following MeTTa script
(= (foo) ((py-dot "A\n" __add__) "B"))
(= (bar) "A\nB")
!(println! (foo))
!(println! (bar))
  1. See output

Expected behavior

A
B
A
B
[()]
[()]

Actual behavior

"A\nB"
A
B
[()]
[()]

Additional context
The following assertion

!(assertEqual (foo) (bar))

properly passes, so the problem seems to be with the interaction of println! and py-dot.

@ngeiswei ngeiswei added bug Something isn't working blocker python labels Jan 30, 2025
@Necr0x0Der
Copy link
Collaborator

My guess is that println! is now implemented in Rust, and strings are parsed into Rust strings, so !(println! "A\nB") works consistently. When you call ((py-dot "A\n" __add__) "B") there should be conversions to Python and back. It seems that the backward conversion keeps \n as symbols, which are not parsed. I suppose this on the base of fact that this code works:

(= (foo) ((py-dot "A\n" __add__) "B"))
!((py-atom print) (foo))

So, Python can print strings formed within Python itself well. At the same time, this also works
!((py-atom print) "A\nB").

@Necr0x0Der
Copy link
Collaborator

btw, is it a blocker for deeper reasons, or workaround with (py-atom print) while not solving the issue in general makes it non-blocking?

@ngeiswei
Copy link
Contributor Author

Using the (py-atom print) trick unblocks me for now. Thank you!

@ngeiswei ngeiswei removed the blocker label Jan 30, 2025
@Necr0x0Der
Copy link
Collaborator

Necr0x0Der commented Jan 30, 2025

@ngeiswei
OK, we figured out the issue. The problem is that __add__ doesn't have type definition, and conversion from Python to Rust doesn't work. Thus, Python object is kept as the grounded object, and when it is passed to Rust println, it is not printed as a string, but its repr is printed instead (so it's not a problem with parsing). Earlier it checked for the Python type of the object, but now it verifies the atom type. We will think if we need to check Python types for %Undefined%, when converting grounded atoms from Python to Rust. However, the current solution is that you can actually specify the type in py-dot manually:

(= (foo) ((py-dot "A\n" __add__ (-> String String String)) "B"))
(= (bar) "A\nB")
!(println! (foo))
!(println! (bar))

@Necr0x0Der
Copy link
Collaborator

The latter solution seems better than (py-obj print), because the result obtained with ((py-dot $a __add__ (-> String String String)) $b)) can be used in other grounded functions over strings, which implemented in Rust.

@vsbogd
Copy link
Collaborator

vsbogd commented Jan 30, 2025

Earlier it checked for the Python type of the object, but now it verifies the atom type.

In Python __repr__ method is used to display Python grounded objects. Before #818 is fixed repr incorrectly printed strings unescaped (see this comment and this fix). After the fix it started printing strings escaped. It is is the reason why atoms which contains string are printed escaped now.

Also special handing was added to Rust and Python to get string internal representation (without escaping) when atom type is String. println! uses this conversion and it the reason why ValueAtom without String type set is processed incorrectly. It is displayed using repr by default and thus it is displayed escaped.

Escaped representation is chosen for the strings by default (and as wider case repr is chosen for other atoms) because when we are displaying atom we are trying to keep the representation which can be used by parser to parse the atom back. Thus parsed program which is printed and saved can be parsed again without errors. println! is a separate case when we want string to be printed unescaped. Thus it relies on a String atom type to correctly interpret it.

@vsbogd
Copy link
Collaborator

vsbogd commented Jan 30, 2025

We could introduce different representations for the atoms:

  • one is suitable for parsing
  • another for converting to the string
  • one more for debugging purposes

Currently in Python the only representation is a __repr__ one (except String atom type). In Rust there is Display which is used for parsing and string conversion (except String atom type) and Debug for unit tests and debugging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working python
Projects
None yet
Development

No branches or pull requests

3 participants