noxdafox / clipspy

Python CFFI bindings for the 'C' Language Integrated Production System CLIPS
BSD 3-Clause "New" or "Revised" License
177 stars 32 forks source link

Understanding the Insertion of Instances into Environment #56

Closed dev-89 closed 8 months ago

dev-89 commented 1 year ago

Hi there,

first of all thank you for clipspy and maintaining it, so far I have found it very useful.

I have though a question regarding the insertion of instance objects. So far as I can tell from the code given in values.py on line 152, Instance objects are cast to C-type-structs and inserted into the CLIPS environment. I don't quite understand this behavior, since CLIPS uses Symbols to add references to instances, instead of adding all data. So if I want to add the instance into a CLIPS class I need to case the instance.name into a InstanceName and then add it to the class..

Based on that I would propose a property in the Instance class like

    @property
    def instance_name(self) -> InstanceName:
        """Instance Class name."""
        return InstanceName(self.name)

That would give a more straight forward way for working with the corresponding symbol.

Thanks again and kind regards

noxdafox commented 1 year ago

Hello,

I'm not quite sure I understand the question. Instances are not created from the Python object but through the related class make_instance as per the example in the documentation.

Python objects such as Class Instance or Fact are not meant to be used directly within the code base.

dev-89 commented 1 year ago

Hi there,

I wrote an example (which ended up a bit lengthy) to demonstrate what I mean:

import clips

env = clips.Environment()

first_class = """
(defclass FirstClass 
  (is-a USER)
  (role concrete)
  (pattern-match reactive)
  (slot One)
  (slot Two))
"""

second_class = """
(defclass SecondClass 
  (is-a USER)
  (role concrete)
  (pattern-match reactive)
  (slot firstCl)
"""

rule = """
(defrule my-rule
  (object (is-a FirstClass)
    (name ?first_class_name)
  (object (is-a SecondClass)
    (firstCl ?first_instance)
  (test (eq ?first_class_name ?first_instance))

  =>
  (printout t "My Rule fired!" crlf))
"""

env.build(first_class)
env.build(second_class)
env.build(rule)

defclass = env.find_class("FirstClass")
first_instance = defclass.make_instance("first_name", One=1, Two=2)

defclass = env.find_class("SecondClass")
instance = defclass.make_instance(
    "second_name", firstCl=first_instance
)  # This way the rule will not get activated

If I change the last lines of the code to

from clips import InstanceName

instance = defclass.make_instance(
    "second_name", firstCl=InstanceName(first_instance.name)
)  # This way the rule will activate

the rule will fire. Hence why I proposed a convenience property like instance_name in my first comment, that way it is clearer for the user when the instance data is used and how to use the instance symbol, which is often needed in rules. I hope that can clear the intention described in the first comment.

Kind regards

noxdafox commented 8 months ago

Apologies for the late reply.

This is the intended usage, if you want to pass an instance name, you need to cast it as such with the InstanceName class.

All CLIPSPy objects return strings in place of names and such. There is ambiguity between a CLIPS symbol and a CLIPS string which is very similar as for Lisp languages (usually solved using quoting '). Unfortunately, there is little we can do with that.

Adding convenience methods all around would complicate the API (lots more code, documentation and tests) with little added value.

After all:

instance = defclass.make_instance(
    "second_name", firstCl=first_instance.instance_name
)  

Is not much shorter than:

instance = defclass.make_instance(
    "second_name", firstCl=InstanceName(first_instance.name)
) 

With the difference that the former, requires the developer to know that instance_name returns a specific type, the latter makes it clear there is a type casting in place.

In my opinion, the latter form is preferable as more Pythonic: simple and clear.

dev-89 commented 8 months ago

Thank you for the reply. I think that then clears my question/proposal.