Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

parse grounded operation is badly implemented #655

Closed
Necr0x0Der opened this issue Apr 9, 2024 · 5 comments
Closed

parse grounded operation is badly implemented #655

Necr0x0Der opened this issue Apr 9, 2024 · 5 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers python

Comments

@Necr0x0Der
Copy link
Collaborator

The code is

parseAtom = OperationAtom('parse', lambda s: [ValueAtom(SExprParser(str(s)[1:-1]).parse(Tokenizer()))],
                              ['String', 'Atom'], unwrap=False)

Apparently, the result of parse should not be wrapped into ValueAtom. Also, instead of a default tokenizer, the actual tokenizer from the runner might be more preferable. It would also be nice to add unit tests in test_stdlib.py

@Necr0x0Der Necr0x0Der added bug Something isn't working enhancement New feature or request good first issue Good for newcomers python labels Apr 9, 2024
@DaddyWesker DaddyWesker self-assigned this Apr 10, 2024
@DaddyWesker
Copy link
Contributor

DaddyWesker commented Apr 10, 2024

Okay, so after removing ValueAtom this is what I'm getting after pytest ./tests

platform linux -- Python 3.11.8, pytest-7.3.2, pluggy-1.4.0
rootdir: /home/daddywesker/Metta/dw_fork/hyperon-experimental/python
collected 93 items                                                                                                                                   

tests/test_atom.py ...........................                                                                                                 [ 29%]
tests/test_atom_type.py ...                                                                                                                    [ 32%]
tests/test_bindings.py .......                                                                                                                 [ 39%]
tests/test_custom_space.py .......                                                                                                             [ 47%]
tests/test_environment.py .                                                                                                                    [ 48%]
tests/test_examples.py ........s.                                                                                                              [ 59%]
tests/test_extend.py .....                                                                                                                     [ 64%]
tests/test_grounded_type.py ...s                                                                                                               [ 68%]
tests/test_grounding_space.py .......                                                                                                          [ 76%]
tests/test_load.py .                                                                                                                           [ 77%]
tests/test_metta.py .....                                                                                                                      [ 82%]
tests/test_minecraft.py ..                                                                                                                     [ 84%]
tests/test_minelogy.py ..                                                                                                                      [ 87%]
tests/test_modules.py .                                                                                                                        [ 88%]
tests/test_pln_tv.py .                                                                                                                         [ 89%]
tests/test_run_metta.py .....                                                                                                                  [ 94%]
tests/test_sexparser.py ..                                                                                                                     [ 96%]
tests/test_stdlib.py ..F                                                                                                                       [100%]

====================================================================== FAILURES ======================================================================
______________________________________________________________ StdlibTest.test_text_ops ______________________________________________________________

self = <test_stdlib.StdlibTest testMethod=test_text_ops>

    def test_text_ops(self):
        metta = MeTTa(env_builder=Environment.test_env())
        # Check that (repr (my atom)) == "(my atom)"
        self.assertEqualMettaRunnerResults(metta.run("!(repr (my atom))"),
                                           [[ValueAtom("(my atom)")]])
    
        # Check that (parse "(my atom)") == (my atom)
>       self.assertEqualMettaRunnerResults(metta.run("!(parse \"(my atom)\")"),
                                           [[ValueAtom(E(S("my"), S("atom")))]])

/home/daddywesker/Metta/dw_fork/hyperon-experimental/python/tests/test_stdlib.py:15: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/home/daddywesker/Metta/dw_fork/hyperon-experimental/python/tests/test_common.py:54: in assertEqualMettaRunnerResults
    self.assertTrue(areEqualMettaRunResults(left, right),
E   AssertionError: False is not true : MeTTa results differ: [[(my atom)]] != [[(my atom)]]
============================================================== short test summary info ===============================================================
FAILED tests/test_stdlib.py::StdlibTest::test_text_ops - AssertionError: False is not true : MeTTa results differ: [[(my atom)]] != [[(my atom)]]
====================================================== 1 failed, 90 passed, 2 skipped in 2.80s =======================================================

In my opinion, it is related to #654 Though maybe I'm wrong.

Replacing Tokenizer() with run_context.tokenizer() works fine on its own, of course with following this strings:

@register_atoms(pass_metta=True)
def text_ops(run_context):
   ...

@DaddyWesker
Copy link
Contributor

Another question is - what kind of unittests should I add to test_stdlib?

@Necr0x0Der
Copy link
Collaborator Author

Obviously, this test expects the result of parsing to be wrapped into ValueAtom, which is just wrong, so these unit tests should be fixes together with fixing implementation of parse. As for unit tests, we can keep them in test_text_ops (no need to move them to test_stdlib). But I'd add more tests for parsing, e.g. including grounded atoms (e.g., parsing (A 2 "S")), nested expressions, maybe something additionally

@DaddyWesker
Copy link
Contributor

fixes together with fixing implementation of parse

Yes, my bad. It is done.

@Necr0x0Der
Copy link
Collaborator Author

Complete by #658

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug Something isn't working enhancement New feature or request good first issue Good for newcomers python
Projects
None yet
Development

No branches or pull requests

2 participants