trueagi-io / hyperon-experimental

MeTTa programming language implementation
https://metta-lang.dev
MIT License
148 stars 49 forks source link

Changes so dasgate metta mod will be loaded without additional config #554

Closed luketpeterson closed 9 months ago

luketpeterson commented 9 months ago

Here is a hotfix so !(extend-py! das_gate) will work right out of the box. When the module system is integrated then !(import! das_gate) will do the same thing.

NOTE the hyperon_das python module must still be loaded, as that is imported by das_gate.py

Necr0x0Der commented 9 months ago

Thanks. This looks similar to what I was thinking about, but I wasn't sure if runner.py is the best place to add the path to builtin libraries (it can be put into extend-py implementation as well). And do we really need this or we can just use (extend-py! hyperon.exts.das_gate). That's why the issue #548 was only about moving das_gate to a dedicated folder. Nothing else! But I guess it's ok this way as well. Instead of metta_mods, I was thinking about exts since we have ext.py for enabling such extensions, but it doesn't matter too much especially if it is a temporary solution (OTOH, we will still need to keep such packages somewhere, so it can be not that temporary). We don't need to rename dasgate to das_gate if we put __init__.py there (or we can just put dasgate.py into exts / metta_mods). And in this case we may not need this hp.env_builder_add_include_path(env_builder, os.path.join(builtin_mods_path, 'das_gate/')). In general, I would try avoiding this hack especially since we want to have various extensions there - not just das-gate. And the test can be created for an extension, which doesn't require installation of specific packages. All in all, I'd prefer reworking it by myself a little, and coordinate with WIP on das-gate to avoid conflicts.

Necr0x0Der commented 9 months ago

PS Python modules with __init__.py don't work atm for paths added via env_builder, but there are at least two ways to fix this. Since we need this functionality now, I'd prefer to fix this together with moving dasgate without waiting for the new module system.

Necr0x0Der commented 9 months ago

555 - this is my alternative. I have not yet put dasgate to exts, but I will do this in coordination with @CICS-Oleg . it fixes also additional issues with loading modules from paths, and it is not limited to das_gate only. I'm ok with renaming exts to metta_mods if it is really preferable. I'm also ok with moving hyperon.exts to env_builder, but I find it less preferable.

luketpeterson commented 9 months ago

Instead of metta_mods, I was thinking about exts since we have ext.py...

I'd also prefer to settle on a name we can keep long term. If you prefer "exts" then that is fine with me. I went with the name "mods" because everywhere else in the documentation I call them "modules" or "mods". In the case of a python-implemented MeTTa module, it is also a Python module, so that might get confusing.

And in this case we may not need this hp.env_builder_add_include_path(env_builder, os.path.join(builtin_mods_path, 'das_gate/')). In general, I would try avoiding this hack especially since we want to have various extensions there - not just das-gate.

The "das-gate" path special case hack is just because the module system isn't merged. When the module system is merged it will use the whole-directory module format code. So this is just a temporary band-aid so the extend-py code finds and recognizes the python file.

luketpeterson commented 9 months ago

https://github.com/trueagi-io/hyperon-experimental/pull/555 - this is my alternative.

Your proposal is fine... My concern is what it implies for the MeTTa module system more generally. Specifically with what I implemented, the module namespace is flat. I don't think it makes sense to delegate the module name resolution to Python. So that means is that MeTTa would need a hierarchical module namespace, and that all Python modules would need to be treated like empty MeTTa modules for the purpose of resolving sub-modules.

I think that would all work fine. But I want to try it out to be sure.

Necr0x0Der commented 9 months ago

The "das-gate" path special case hack is just because the module system isn't merged.

My point is that this special hack is not needed specifically for das-gate, but can work for any module placed in this dedicated folder.

I don't think it makes sense to delegate the module name resolution to Python.

I'm ok with this statement, although it depends on how it will work in practice. For example, in some cases, we would like a certain module to work simultaneously as a normal Python package (installed via pip) and a normal MeTTa module/extension (which will be found in the same location as Python package installed via pip). In particular, it will be true for AI-DSL/MeTTa SDK for SNet platform, or Metta-Motto. In these cases, we will need to delegate the module name resolution to Python, although we will not need to always rely on it.

luketpeterson commented 9 months ago

My point is that this special hack is not needed specifically for das-gate, but can work for any module placed in this dedicated folder.

When the modules branch is merged, no path hack will be needed for dasgate or any other module. The module loading code has a ModuleFormat that knows modules can take the form of directories. So this is a very short-term issue.

I'm ok with this statement, although it depends on how it will work in practice. For example, in some cases, we would like a certain module to work simultaneously as a normal Python package (installed via pip) and a normal MeTTa module/extension (which will be found in the same location as Python package installed via pip). In particular, it will be true for AI-DSL/MeTTa SDK for SNet platform, or Metta-Motto. In these cases, we will need to delegate the module name resolution to Python, although we will not need to always rely on it.

100 percent agreed. The way it works is there is an object called a Catalog that is part of the module name resolution process. There is a Catalog for Python modules, so they can be loaded as MeTTa modules.

What is missing is namespace resolution. Right now the '.' character is illegal in a module name. Because I thought we might want it as a delimiter in the future. I'm not sure if then future has come already.

Necr0x0Der commented 9 months ago

When the modules branch is merged, no path hack will be needed for dasgate or any other module.

But until it is not merged, the right hack is needed.

luketpeterson commented 9 months ago

Closing as https://github.com/trueagi-io/hyperon-experimental/pull/555 addresses what this intended to address.