search

trueagi-io / hyperon-experimental

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

Add doc strings related functions #640

Closed vsbogd closed 3 months ago

vsbogd commented 3 months ago

Work on #321.

This PR is to review:

If items above are approved next steps are:

What is done

Add doc strings to if, add-atom, ErrorType standard library functions as example. Add get-doc which compiles documentation as a doc-formal atom from doc strings and type information. Add help! to print doc-formal in a human friendly way. Add format-args function to format strings inside help!. Add quick implementation for a String type in order to remove double quotes around printed strings.

vsbogd commented 3 months ago

format-args implementation is also topic for a discussion. I used dyn-fmt library because strfmt requires named argument positions inside format string. We can do it using something like: (format-args "{first} {second}" ((first <first-atom>) (second <second-atom>))) for now I used arguments without names: (format-args "{} {}" (<first-atom> <second-atom>))

vsbogd commented 3 months ago

So merging this is fine. But I may replace the dyn-fmt crate in the near future unless you object.

Yeah, I used dyn-fmt because I didn't want to construct a hash map and assign names to the arguments on the first place. But I don't think dyn-fmt is a best approach. I absolutely open for any good replacements, including strfmt if it supports all necessary modes.

vsbogd commented 3 months ago

One relatively minor question: Should we replace long names: "description", "parameters", "parameter" by their shorter versions: "descr", "params", "param"? Or should we hope that auto-completion will make typing documentation easier with long names?

luketpeterson commented 3 months ago

One relatively minor question: Should we replace long names: "description", "parameters", "parameter" by their shorter versions: "descr", "params", "param"? Or should we hope that auto-completion will make typing documentation easier with long names?

Personally I feel like 4-7 characters is the sweet spot for these kind of names. Long enough to be visually distinct, short enough not to waste space on the screen & elsewhere. So "descr", "param", etc. is where I vote.

Also, unless you feel differently, I think it would be good to adopt the keywords from another system where our keywords do the same thing. We obviously don't need to copy every feature, but where we implement the same feature, we could use the same keyword.

For example, doxygen's are here. https://www.doxygen.nl/manual/commands.html But I'd be just as happy with any doc system. I don't especially love doxygen.

vsbogd commented 3 months ago

Also, unless you feel differently, I think it would be good to adopt the keywords from another system where our keywords do the same thing.

Yeah, I agree with this, but I tried to copy names from metta-motto examples provided by @Necr0x0Der. @Necr0x0Der , do you mind some renaming? Which names do you prefer?

luketpeterson commented 3 months ago

I actually don't feel strongly. I just thought we could leverage the documentation and expectation of others if we copied their names. But it's not a big deal at all.

Necr0x0Der commented 3 months ago

do you mind some renaming?

I don't have strong opinion here

vsbogd commented 3 months ago

Not merging it because after writing unit tests I have found the issue explained in https://github.com/trueagi-io/hyperon-experimental/issues/354#issuecomment-2024809298 I am trying to figure out how to resolve it properly.

luketpeterson commented 3 months ago

From #354

I am thinking about adding &top to the stdlib...

With the fix in https://github.com/trueagi-io/hyperon-experimental/pull/643 you can access !(mod-space top) without changing the stdlib. I know that's not a fix for #354, but slightly more convenient than adding to stdlib.

I'm thinking about the best solution for #354 in parallel.

vsbogd commented 3 months ago

I added get-type-space operation see commit https://github.com/trueagi-io/hyperon-experimental/pull/640/commits/06d0901ddbd6441b2e04c89f7441f37562d958b2 and used it to get type using top level space.

Always using top level space in get-type doesn't work for the scripts like g1_gadt.metta because they are executed using metta.load_module_at_path (see test_run_metta.py) and during this execution top space doesn't include current working space. Thus get-type cannot find the type of the atom if type is defined in a space being loaded.

Commits https://github.com/trueagi-io/hyperon-experimental/pull/640/commits/72faef58f2f081b3cf7609481b2cb4697975b466 and https://github.com/trueagi-io/hyperon-experimental/pull/640/commits/06d0901ddbd6441b2e04c89f7441f37562d958b2 are used to implement unit tests in https://github.com/trueagi-io/hyperon-experimental/pull/640/commits/aee5eea650be9ee2042f3dea5dc645ec00d83b6a

vsbogd commented 3 months ago

Commit https://github.com/trueagi-io/hyperon-experimental/pull/640/commits/7f0a4aeb7841c6b16b8c390eef083d157d3e560c adds doc related functions into a minimal MeTTa stdlib.

vsbogd commented 3 months ago

CI failed because one of the workflow modules was unavailable.

vsbogd commented 3 months ago

Last problem with this PR is that return is actually internal symbol in minimal MeTTa. This means if it type-checks than it is evaluated. Thus having (doc ... (return <something>)) forces the evaluation of the (return <something>).

On the one hand this is a problem of aliasing. We have two different returns docs:return and minimal:return and we could use aliases to distinguish them (for instance using "import as" from docs modules). But as we don't have aliasing right now we need another solution.

One possible solution is to rename "minimal return" or "docs return". For example use short form "ret" for documentation. Or add a prefix like @return to the documentation tags.

Another possible solution still use return but use Atom instead of DocReturn type inside doc and doc-formal types.

I like the renaming option more because I feel these two return are different symbols effectively. @Necr0x0Der @luketpeterson what do you think?

Necr0x0Der commented 3 months ago

ret or @return sounds good to me

luketpeterson commented 3 months ago

ret or @return sounds good to me

No strong opinion on the symbol. Although @return is distinct in a way that we should probably maintain across the other doc-associated keywords/ symbols. So my vote is for consistency.

On the one hand this is a problem of aliasing. We have two different returns docs:return and minimal:return and we could use aliases to distinguish them (for instance using "import as" from docs modules). But as we don't have aliasing right now we need another solution.

It's an interesting idea to extend the symbol space into the module space. I certainly thought this is a direction we might want to go. And we absolutely could. But currently we don't have anything like this. It would involve making ':' a reserved character in symbols to avoid ambiguity.

vsbogd commented 3 months ago

It would involve making ':' a reserved character in symbols to avoid ambiguity.

JIC, I used : only as an example, we can use anything appropriate.

Although @return is distinct in a way that we should probably maintain across the other doc-associated keywords/ symbols.

Yeah, I think it is worse to use @ prefix for all doc symbols then.

luketpeterson commented 3 months ago

It would involve making ':' a reserved character in symbols to avoid ambiguity.

JIC, I used : only as an example, we can use anything appropriate.

':' would make sense since it's the separator module names use. So it would be consistent. But adding "full-path-symbols" a pretty big change to introduce before Alpha because it would likely need interpreter coordination to implement.

vsbogd commented 3 months ago

Implemented showing docs for the functions without explicit types https://github.com/trueagi-io/hyperon-experimental/pull/640/commits/21faf744df4459c336c3f76eaf621b93dda89206. Renamed all doc tags to the versions with @ prefix https://github.com/trueagi-io/hyperon-experimental/pull/640/commits/3fdd12c0385823dc9ab5bd0121f5635f9089020a. Doc tags has very common wording thus I think it is better to make them unique in order to eliminate possible conflicts.

vsbogd commented 3 months ago

I don't see any other functional issues with this PR. One non-functional issue is that unit test for get-doc takes a lot of time for the minimal interpreter version.

vsbogd commented 3 months ago

Ignored tests in https://github.com/trueagi-io/hyperon-experimental/pull/640/commits/1d41ab1190d16f3b52359abf14f3c0b170ff8e08 raised https://github.com/trueagi-io/hyperon-experimental/issues/660 to fix it.