russellmcdonell / pyDMNrules

An implementation of DMN (Decision Model Notation) in Python
Other
38 stars 9 forks source link

fix: not in simple list #3

Closed lnu closed 2 years ago

lnu commented 3 years ago

WIP

russellmcdonell commented 3 years ago

I'm not sure the issue is in pyDMNrules. pyDMNrules depends up pySFeel and there were some bugs in pySFeel, which have been addressed recently. in() a 'list' was one of those bugs. See if you can upgrade your version pySFeel and try again. I am currently running pySFeel verion 0.1.11a0 and cannot replicate you issue.

russellmcdonell commented 2 years ago

I've just release version 1.2.0 of pySFeel and version 1.2.0 of pyDMNrules. Both have bug fixes. Both have unit test so I can do regression testing of any future releases. However, 'in' was one of the things changed. A simple comma separated list now becomes 'variable in(list)' - it uses the S-FEEL in() function which treats each thing in the list as an idividual unary test - with the default operator being equality. If any test passes then the in() function returns true. So you can have <10,>12 and it will do 'variable < 10' and 'variable >12' and if either are true, then the input test is true. See AR-SNAP rules (DMN).xlsx in github for a complex example.

lnu commented 2 years ago

With the latest version I have this: image

If I put something like this: image

It throws a bad syntax error, is the syntax wrong?

And for the in, in 300,400,500 was working with the previous version but now it's either 300,400,500 or in(300,400,500), otherwise it fails.

lnu commented 2 years ago

Aside from that I face another issue. We have an expression like this XXX in Container.relations where Container.relations is a variable like this ["AAA","BBB","XXX"] (Container is the context, relations the attribute). It was working with the previous version but doesn't work anymore with the latest one. The expression is wrongly translated to XXX in Container.Container.relations in data2sfeel. I did some more tests: if I put back the old code for data2sfeel, our existing DMN works fine. This code causes the issue:

        # Check that a string of text (data) is valid S-FEEL
        # Start by replacing all 'Variable's with their BusinessConcept.Attribute equivalents (which are valid S-FEEL)
        # Being careful not to replace any BusinessConcept.Attributes that already exist in data
        text = data
        for variable in self.glossary:
            item = self.glossary[variable]['item']
            if variable == self.glossary[variable]['concept']:       # The variable and it's business concept share the same name
                at = 0
                match = re.search(variable, text[at:])
                while match is not None:
                    replaceIt = True
                    if match.end() == len(text[at:]):
                        pass
                    elif text[at + match.end():at + match.end() + 1] != '.':
                        pass
                    else:
                        for otherItem in self.glossaryItems:
                            if text[at:].startswith(otherItem):
                                replaceIt = False
                                break
                    if replaceIt:
                        text = text[:at] + text[at:].replace(variable, item, 1)
                    at += match.end()
                    match = re.search(variable, text[at:])
            else:
                text = text.replace(variable, item)
russellmcdonell commented 2 years ago

Yes, I understand why. There was a bug that you were exploiting ...

not is both a unary operator and a function. So not x and not(x) do the same thing - return 'true or 'false, depending upon x - assuming x is a boolean value.

in is both a unary operator and a function, BUT ... in is only a valid operator when the following expression is a string or a FEEL List (i.e. a comma separted list of values enclosed in []) The in() function takes, as it's parameters, a set of unary tests. So in [300,400,500] is value as it will become Input.value in [300,400,500]

and in(=300,=400,=500) is also valid as it become Input.value = 300 OR Input.value = 400 OR Input.value = 500

Now S-FEEL assumes that any missing operator in an in() function parameter implies '='. So in(300,400,500) is also valid as it also becomes Input.value = 300 OR Input.value = 400 OR Input.value = 500

NOTE: in(<300,!=400,>500) is also valid, but in [<300,!=400,>500] is invalid

Hope that helps.

Russell McDonell

On Wed, 13 Oct 2021 at 16:45, Laurent Nullens @.***> wrote:

With the latest version I have this: [image: image] https://user-images.githubusercontent.com/1829553/137074065-bfcf4c8b-285b-4aa9-b6d4-042b004fd655.png

If I put something like this: [image: image] https://user-images.githubusercontent.com/1829553/137074093-d78a4bf9-d823-4e25-a224-a21d72e095a8.png

Is the syntax wrong?

And for the in, in 300,400,500 was working with the previous version but now it's either 300,400,500' or in(300,400,500)`, otherwise it fails.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/russellmcdonell/pyDMNrules/pull/3#issuecomment-941941942, or unsubscribe https://github.com/notifications/unsubscribe-auth/AECWZESFAKAU7D5BVTGL5ETUGUMIBANCNFSM473Z5EKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

russellmcdonell commented 2 years ago

And Container.Container.relations was a bug in data2sfeel() - fixed in release 1.2.2 - which is not up on PyPi.

Russell McDonell

On Wed, 13 Oct 2021 at 21:25, Laurent Nullens @.***> wrote:

Aside from that I face another issue. We have an expression like this XXX in Container.relations where Container.relations is a variable like this ["AAA","BBB","XXX"]. It was working with the previous version but doesn't work anymore with the latest one. The expression is wrongly translated to XXX in Container.Container.relations in data2sfeel.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/russellmcdonell/pyDMNrules/pull/3#issuecomment-942154027, or unsubscribe https://github.com/notifications/unsubscribe-auth/AECWZEWSVS6WLD32EAPCIMDUGVM7XANCNFSM473Z5EKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

lnu commented 2 years ago

When do you plan to release the new version?

russellmcdonell commented 2 years ago

Hum, thought I released pyDMNrules 1.2.2 about 9 hours ago?

Russell McDonell

On Thu, 14 Oct 2021 at 19:25, Laurent Nullens @.***> wrote:

When do you plan to release the new version?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/russellmcdonell/pyDMNrules/pull/3#issuecomment-943130661, or unsubscribe https://github.com/notifications/unsubscribe-auth/AECWZEUAIMUFMP4OYH3NC5LUG2HY7ANCNFSM473Z5EKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

lnu commented 2 years ago

sorry my bad... It's on pypi. Thank you.

russellmcdonell commented 2 years ago

That's O.K. I was expecting you to point out that I'd stuffed up, that being the more likely scenario. Your testing seems to be better than my coding - so keep at it.

I've had a brief look at DMN 1.3 and the changes to pySFeel/pyDMNrules. Don't look all that hard, but I'd really like to bed down 1.2 first. I do use it, at FHIR connectatons, but my usage isn't extensive, and you tend to repeatedly use the thing that worked last time. Hence the interest in developing a wider community of usage and more test cases.

Russell McDonell

On Thu, 14 Oct 2021 at 22:38, Laurent Nullens @.***> wrote:

sorry my bad... It's on pypi. Thank you.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/russellmcdonell/pyDMNrules/pull/3#issuecomment-943274570, or unsubscribe https://github.com/notifications/unsubscribe-auth/AECWZEWL4KL7U5TXQEISSSDUG26M3ANCNFSM473Z5EKQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

russellmcdonell commented 2 years ago

I have just released pySFeel version 1.3.9 and pyDMNrules version 1.3.18. Both have fixes but I believe you issue was fixed several releases ago. So I am closing this pull request.