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

Various fixes on ValueConstraint comparison & 1e-6 support #13

Merged
merged 6 commits into from
Mar 5, 2024

Conversation

andyward
Copy link
Collaborator

@andyward andyward commented Mar 5, 2024

  1. When BaseType is undefined for ExactConstraint the (int)42 == (long)42 would fail despite obvious implicit casting available if types were known.
    BaseType is often undefined. E.g. Attributes don't have a datatype in the XSD in the same way Psets can
  2. Minor tidying of some typos, and improved naming of some functions
  3. Added support for treating accented strings as equal when using ignoreCase = true. E.g. "à rénover"=="A Renover"
    ignoreCase is clearly an extension to the standard, but this feels like a useful addition.
  4. Added support for 1e-6 tolerance on Reals when testing equality with ExactConstraint. So 42 == 42.000042d, but 42 != 42.000084d
    This is based on the algorithm at Testcase "[PASS] Floating point numbers are compared with a 1e-6 tolerance 1-4/4" buildingSMART/IDS#78 (comment)
  5. Extended the 1e-6 to RangeConstraints
    This last one may need some discussion with other implementors as it introduces some subtleties, such as

42.00001 <= 42 : Constraint Satisfied - despite being > 42
42.00001 > 42 : Constraint Satisfied - as expected

andyward added 6 commits March 4, 2024 17:16
…use BaseType was undefined.

Renamed TypeNames parsing functions to BETTER indicate their intent/behaviour (left forwarding Obsolete markers) on public interfaces
Also fixed 'candidate' typo
Case insensitive ExactConstraint comparisons now ignore accents
Supports Float/Double equality checks only.
TODO: Range checking
Only takes effect when using Inclusive ranges.
Likely needs review with wider IDS community
… issues in other locales.

E.g. 356.789 in italian might be interpretted as 356 thousand-odd, not 356 point 789
Main one being -ve Reals!
Better handles extremes.
@CBenghi
Copy link
Owner

CBenghi commented Mar 5, 2024

Hi @andyward,
are you still working on this or should I merge?

@andyward
Copy link
Collaborator Author

andyward commented Mar 5, 2024 via email

@CBenghi CBenghi merged commit 5000a9a into main Mar 5, 2024
1 check passed
andyward added a commit that referenced this pull request Mar 6, 2024
Fixed Range regression with Ints from related commits #13
@andyward andyward deleted the feature/value-fixes branch March 26, 2024 14:48
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

Successfully merging this pull request may close these issues.

2 participants