Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Invalid special case for Di in Transition #74

Open
MaximPlusov opened this issue May 26, 2023 · 7 comments
Open

Invalid special case for Di in Transition #74

MaximPlusov opened this issue May 26, 2023 · 7 comments

Comments

@MaximPlusov
Copy link
Contributor

MaximPlusov commented May 26, 2023

Special case for Di in Transition is currently fn:Eval((((@Di==90) || (@Di==180)) && (@S==Wipe)) || ((@Di==315) && (@S==Glitter)))
Possible values for entry Di: 0,90,180,270,315.
If Di have value 0 or 270, value of this special case is false.

Possible correct variants:
1.fn:Eval((((@Di==90) || (@Di==180)) && (@S==Wipe)) || ((@Di==315) && (@S==Glitter)) || (@Di==0) || (@Di==270))

2.fn:Eval((((@Di!=90) && (@Di!=180)) || (@S==Wipe)) && ((@Di!=315) || (@S==Glitter)))

@petervwyatt
Copy link
Member

The intention is to combine the "PossibleValue" field with the "SpecialCase" field, so as to avoid having to duplicate every PossibleValue into every SpecialCase throughout the entire data model.
In this case, Di values 0 and 270 have no specific additional conditions so they don't need to be in SpecialCase.

@MaximPlusov
Copy link
Contributor Author

Another example of 'invalid' special case: Object Collection, entry View, special case fn:Eval(fn:SinceVersion(2.0,((fn:IsEncryptedWrapper() && (@View==H)) || (fn:IsPresent(Navigator) && (@View==C)) || (@View!=C))))
If View == H, but fn:IsEncryptedWrapper() == false, value of fn:IsPresent(Navigator) && (@View==C)) || (@View!=C)) would be true, and value of whole predicate also would be true.

I found only these 2 special cases (for Di and View) with different conditions for different possible values.
Maybe these 2 cases are understandable and convenient for people, but not for machines. Such cases are not easy to recognize and

combine the "PossibleValue" field with the "SpecialCase" field.

  1. I think a new ‘conditional’ predicate is a possible solution. fn:new_predicate_name(expr1, expr2): when cond1 is true, that cond2 should be true.
    For example:
    fn:Eval((( (@Di==90) || (@Di==180) ) && (@S==Wipe) ) || ( ( @Di==315 ) && ( @S==Glitter ))) =>
    fn:Eval(fn:new_predicate_name((@Di==90) || (@Di==180), (@S==Wipe)) && fn:new_predicate_name (@Di==315, @S==Glitter))

  2. Other option: transform these special cases to required predicate in "Possible values" field for Wipe and Glitter values of S for Transition and for C and H values of View.

@petervwyatt
Copy link
Member

That is not a bad idea and in line with a few other existing predicates such as fn:DefaultValue(condition,value), fn:IsPresent(key,condition), and fn:RequiredValue(condition,value). However I still want predicates to read aloud nicely when read from left-to-right and I want to avoid any allusions to functional programming so maybe fn:ValueOnlyWhen(value,condition) and use this in the PossibleValues field:

  • fn:ValueOnlyWhen(90,(@S==Wipe)) - read aloud as "value can be 90 but only when S is Wipe" (and which is NOT the same as saying "value MUST BE 90 but only when S is Wipe" since that would be incorrect!)
  • fn:ValueOnlyWhen(180,(@S==Wipe))
  • fn:ValueOnlyWhen(315,(@S==Glitter))
    and for the name condition:
  • fn:ValueOnlyWhen(None,((@S==Fly) && (@SS!=1.0)))

Note also that for name None this is explicitly stated as the only allowable name and relelvant only for Flt so it really should be expressed as: fn:RequiredValue(((@S==Fly) && (@SS!=1.0)),None) since this is a stronger statement than fn:ValueOnlyWhen

So PossibleValues would change from:
[0,90,180,270,315];[None]
to
[0,fn:ValueOnlyWhen(90,(@S==Wipe)),fn:ValueOnlyWhen(180,(@S==Wipe)),270,fn:ValueOnlyWhen(315,(@S==Glitter)))];[fn:RequiredValue(((@S==Fly) && (@SS!=1.0)),None)]

This is also more compact since @Di== is not needed multiple times in PossibleValues (as it is implicit)

and SpecialCase would then be empty for Transition.tsv.

This also avoids the very programmatic idioms needed to hande all the OR cases in SpecialCases (such as when Di==0 or Di==270).

@petervwyatt petervwyatt reopened this May 28, 2023
@MaximPlusov
Copy link
Contributor Author

fn:ValueOnlyWhen is great idea.

@petervwyatt petervwyatt added this to the PDF Data Model milestone Mar 16, 2024
@petervwyatt
Copy link
Member

@MaximPlusov - do you have a test PDF for this?

@MaximPlusov
Copy link
Contributor Author

I don't have such tests files, but if you need I can create them

@petervwyatt
Copy link
Member

It's OK - I can create too. Thought you might have one handy...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants