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

UPPERCASE instead of PascalCase in IDS? #132

Open
daviddelven opened this issue Mar 11, 2023 · 9 comments
Open

UPPERCASE instead of PascalCase in IDS? #132

daviddelven opened this issue Mar 11, 2023 · 9 comments
Assignees
Labels
documentation Improvements or additions to documentation
Milestone

Comments

@daviddelven
Copy link

Is there some reason to keep UPPERCASE instead of PascalCase for a class identification within the IDS specification, as shown in the Candidate Standard Report - January 2023?

image

@daviddelven daviddelven changed the title Uppercase instead of PascalCase in IDS? UPPERCASE instead of PascalCase in IDS? Mar 11, 2023
@aothms
Copy link

aothms commented Mar 11, 2023

There might be applications that are not schema-aware and only can go by the instance definitions from the .ifc file which is uppercase. I'm not sure how realistic that is, but that is what we discussed at the time.

@berlotti
Copy link
Member

That was indeed the reason for the decision.

@SergejMuhic
Copy link

What about ifcxml?

@janbrouwer
Copy link
Contributor

I would at any time vote for PascalCase over uppercase when strings are concatenated parts. Any software can go from PascalCase to UPPERCASE but the reverse is not possible without consulting a schema. I believe it's even more important for predefined types then for entities.

@pjanck
Copy link
Contributor

pjanck commented Mar 17, 2023

@janbrouwer +1 from me!

@TLiebich
Copy link

the IFC specification for identifiers in EXPRESS is indeed not case sensitive, in ifcXML however it is. Also the candidate spec says that IDS should be applicable in principle also to other schemas (maybe gml, gbXML?) - and then all UPPERCASE ?

so yes, +1 from me as well.

@CBenghi
Copy link
Contributor

CBenghi commented Mar 17, 2023

Because of regexes in the patterns, implementers are going to have quite a bit of work to do, with any of the choices:

  • for UX developers to produce a good readable UI,
  • for implementers to consider all the XML/EXPRESS options.
  • for the users to ensure that the files we exchange are meaningful and valid across various UXs/Checkers.

Just making strings UPPERCASE when saving from the UI is not enough, because we will have to include some logic to tell apart the metacharacters of regexes:

E.g.:

\w Matches any word character including underscore. This expression is equivalent to [A-Za-z0-9_].
\W Matches any non-word character. This expression is equivalent to [^A-Za-z0-9_].

PascalCase carries more information (PC to UC can be done, not vice versa), but that is also more error prone if end users are typing freely (especially in regexes), so UPPERCASE will probably result in fewer invalid IDS files.

@aothms, I think that the argument for applications that are not schema-aware, is weaker than it was in the past, since we have introduced attributeType, inheritance of properties through types, measures conversion and more.

The choice we make, I think, should depend on what makes most resilient ecosystem, from an end user perspective.

Personally, I think that making the tests case-insensitive would deliver the best user experience, but I acknowledge that it would be the most complex implementation, particularly for UIs.

@aothms
Copy link

aothms commented Mar 17, 2023

@aothms, I think that the argument for applications that are not schema-aware, is weaker than it was in the past

I wouldn't disagree with that, I was thinking the same actually.

Personally, I think that making the tests case-insensitive would deliver the best user experience

That is also most inline with the express semantics. Express is case-insensitive.

I think most regex implementations have an option/flag to ignore case [0] [1]. So you wouldn't have to do case normalization on the pattern (potentially triggering changes when doing \W -> \w.)

I think there is a point for case insensitivity on schema names (what about pset and prop names?) but it's also potentially surprising when then model values are handled case sensitively (users might not always be able to tell the difference).

[0] https://docs.python.org/3/library/re.html#re.IGNORECASE

[1] https://learn.microsoft.com/en-us/dotnet/api/system.text.regularexpressions.regexoptions?view=net-7.0#system-text-regularexpressions-regexoptions-ignorecase

CBenghi added a commit to CBenghi/IDS that referenced this issue Mar 19, 2023
This might change in the future, depending on buildingSMART#132, but for the time
being `IfcBuildingStorey` is invalid.
CBenghi added a commit to CBenghi/IDS that referenced this issue Mar 20, 2023
This might change in the future,
also depending from a parallel conversation happening on buildingSMART#132,
but for the time being 'name' is not a valid attribute name;
it should be capitalized ('Name').
berlotti pushed a commit that referenced this issue Mar 21, 2023
This might change in the future, depending on #132, but for the time
being `IfcBuildingStorey` is invalid.
berlotti pushed a commit that referenced this issue Apr 5, 2023
* Entity name should be UPPERCASE

This might change in the future, depending on #132, but for the time
being `IfcBuildingStorey` is invalid.

* Attribute name should match case with schema

This might change in the future,
also depending from a parallel conversation happening on #132,
but for the time being 'name' is not a valid attribute name;
it should be capitalized ('Name').

* Fixed documentation samples.

- Uppercase for types in enumeration
- Mismatch of case for 'Name' (was 'name')
- totalDigits is not valid for xs:string (use length instead)
- formatting of xs:annotation

* index on allPr: 612d376 Merge branch 'pr/invalidEntityName' into allPr

* Fixed min and max Occurrence id Development.

* Development samples are all ok.
@CBenghi CBenghi added this to the 1.0 milestone May 31, 2023
@CBenghi
Copy link
Contributor

CBenghi commented Oct 10, 2023

The group decided that for simplicity we can keep it UPPERCASE for version 1.0.
This will need to be documented appropriately, and we can investigate if it is possible to add a restriction to the xsd schema.

@CBenghi CBenghi added documentation Improvements or additions to documentation and removed discuss & decide labels Oct 10, 2023
@CBenghi CBenghi self-assigned this Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

8 participants