Open ChrisIsKing opened 1 week ago
Hey, I may not have time to knock out a pr for this but its very easy:
In this piece of code below just move the walker entry and exit blocks outside the loop:
These are the blocks that should be before and after loop:
@ChrisIsKing Does kugesan's pr on this resolve your issue?
@marsninja @kugesan1105 I think I found a bug, not sure if this is related to the PR but I modified the test case to check for the condition where the developer wants to have an ability that is triggered on any node entry
can log_visit with `Any exit {
print("Visiting node : ", here);
self.visited_nodes.append(here);
}
It resulted in this error:
Entering at the beginning of walker: Root()
ERROR - Error: typing.Any cannot be used with isinstance()
raise TypeError("typing.Any cannot be used with isinstance()")
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
at __instancecheck__() /usr/local/Cellar/python@3.12/3.12.5/Frameworks/Python.framework/Versions/3.12/lib/python3.12/typing.py:530
at spawn_call() /Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/runtimelib/architype.py:558
at spawn_call() /Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/plugin/default.py:371
at _multicall() /Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/vendor/pluggy/_callers.py:103
at _multicall() /Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/vendor/pluggy/_callers.py:139
at _hookexec() /Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/vendor/pluggy/_manager.py:120
at __call__() /Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/vendor/pluggy/_hooks.py:513
at spawn_call() /Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/plugin/feature.py:168
at <module> /Users/chrisisking/dev/jaseci/jac/jaclang/tests/fixtures/entry_exit.jac:34
Traceback (most recent call last):
File "/Users/chrisisking/dev/jaseci/venv/bin/jac", line 8, in <module>
sys.exit(start_cli())
^^^^^^^^^^^
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/cli/cli.py", line 514, in start_cli
ret = command.call(**args_dict)
^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/cli/cmdreg.py", line 25, in call
return self.func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/cli/cli.py", line 91, in run
jac_import(
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/plugin/feature.py", line 119, in jac_import
return pm.hook.jac_import(
^^^^^^^^^^^^^^^^^^^
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/vendor/pluggy/_hooks.py", line 513, in __call__
return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/vendor/pluggy/_manager.py", line 120, in _hookexec
return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/vendor/pluggy/_callers.py", line 139, in _multicall
raise exception.with_traceback(exception.__traceback__)
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/vendor/pluggy/_callers.py", line 103, in _multicall
res = hook_impl.function(*args)
^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/plugin/default.py", line 268, in jac_import
import_result = JacImporter(JacMachine.get()).run_import(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/runtimelib/importer.py", line 354, in run_import
raise e
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/runtimelib/importer.py", line 351, in run_import
exec(codeobj, module.__dict__)
File "/Users/chrisisking/dev/jaseci/jac/jaclang/tests/fixtures/entry_exit.jac", line 34, in <module>
wlk_obj = root spawn test_walker();
^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/plugin/feature.py", line 168, in spawn_call
return pm.hook.spawn_call(op1=op1, op2=op2)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/vendor/pluggy/_hooks.py", line 513, in __call__
return self._hookexec(self.name, self._hookimpls.copy(), kwargs, firstresult)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/vendor/pluggy/_manager.py", line 120, in _hookexec
return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/vendor/pluggy/_callers.py", line 139, in _multicall
raise exception.with_traceback(exception.__traceback__)
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/vendor/pluggy/_callers.py", line 103, in _multicall
res = hook_impl.function(*args)
^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/plugin/default.py", line 371, in spawn_call
return op2.__jac__.spawn_call(op1.__jac__)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/chrisisking/dev/jaseci/venv/lib/python3.12/site-packages/jaclang/runtimelib/architype.py", line 558, in spawn_call
if not i.trigger or isinstance(current_node, i.trigger):
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/Cellar/python@3.12/3.12.5/Frameworks/Python.framework/Versions/3.12/lib/python3.12/typing.py", line 530, in __instancecheck__
raise TypeError("typing.Any cannot be used with isinstance()")
TypeError: typing.Any cannot be used with isinstance()
Since this addition replaces the ability to trigger on an entry of any type, we should still cater for this.
Seems like you can achieve this using the object
type.
can log_visit with object exit {
print("Visiting node : ", here);
self.visited_nodes.append(here);
}
All nodes are python object so it makes sense that this works but there is a readability argument here as core jac users may not be familiar with this keyword.
@marsninja If you're fine with the above comments, we can close this issue.
@ChrisIsKing and @kugesan1105, Actually this is actually and important issue for us to address in the language. Essentially a number of legit types from the typing
library cannot be used with isinstance and thus will break similarly. We definitely need a PR that adds a test of this issue where we have better error reporting.
@kugesan1105 The best way to build this static error condition (that would trigger in the static compiler passes) is to look into the below code location in pythons standard library and see all teh casese that trigger this type of error and statically try to catch them during compile.
File "/usr/local/Cellar/python@3.12/3.12.5/Frameworks/Python.framework/Versions/3.12/lib/python3.12/typing.py", line 530, in __instancecheck__
raise TypeError("typing.Any cannot be used with isinstance()")
Spool up a pr with the test and your (or someone else on the team) should look into whether that fix makes sense. Certainly keep me posted.
Also, @ChrisIsKing Thank goodness you didn't close this and move on with the work around. These are exactly the types of things we need to do to button up Jac. object is the way to go here but better error reporting is super necessary and shouldnt fall throught eh cracks.
@marsninja My understanding of the with entry
and with exit
blocks is that they're core data spatial abilities that trigger with graph traversal. These graph traversal triggers are only relevant to nodes
and walkers
. As such why not modify the language only to allow objects of type node
or walker
to trigger for a with some_object entry/exit
. Leaving it up to any type supported by isinstance()
allows devs to create weird abilities like for e.g
walker test_walker {
can print_something with int entry {
print("Something:", f'{here}');
}
}
with entry {
root spawn test_walker();
}
You are 100% correct @ChrisIsKing , and now that you mention it there should indeed be a typecheck error in the with entry syntax. Right now all of the typechecks are warnings but in principle and when its beta tested enough we should flip a switch to make jac treat these (or at least some of these) as static errors. This would be in the realm of Jac as a typescript to javascript mode (which woudl be an option to make type checks warnigns).
@kugesan1105 do we have a test case specificly for hte kind of code chris shows in his example. This would simply validate that the type check pass produces an error with the with int entry. If not do assign that PR to me once created and i can take a quick look.
Describe the bug
In Jac 1 walkers had the ability to execute functions/abilities based on the node they are currently traversing. e.g
the same can be achieved in jaclang with a more descriptive and readable syntax. eg.
This execute logic when entering a node of type
test_node
.However, missing from jaclang is the ability to execute upon the the
entry
andexit
of the entire walk. e.g in Jac 1 you could doSyntactically, it appear you could the same in jaclang by doing the following
However, this results in the logic being executed upon the entry and exit of any node type, which is not the desired behavior. This should behave like jac1 since the same can already be achieved by doing
can execute_entry with any entry
.To Reproduce
Here is a sample program to reproduce this: