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

Check IfcDateTime according to ISO standards #3864

Open
wants to merge 8 commits into
base: v0.7.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
119 changes: 119 additions & 0 deletions src/ifcopenshell-python/ifcopenshell/type_checker.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
from lark import Lark, Transformer

# Grammar to retrieve the following structure: YYYY-MM-DDThh:mm:ss
grammar = """

datetime_value: year "-" month "-" day "T" hour ":" minute ":" second ["." millisecond]

hour: DIGIT DIGIT
minute: DIGIT DIGIT
second: DIGIT DIGIT
millisecond: DIGIT DIGIT DIGIT*

day: DIGIT DIGIT
month: DIGIT DIGIT
year: MINUS? DIGIT DIGIT DIGIT DIGIT DIGIT*

MINUS: "-"
DIGIT: "0".."9"

%import common.WS
%ignore WS
"""

parser = Lark(grammar, start='datetime_value')


class DatetimeTransformer(Transformer):
def year(self, items):
return int(''.join(token.value for token in items))

def month(self, items):
return int(''.join(token.value for token in items))

def month(self, items):
return int(''.join(token.value for token in items))

def day(self, items):
return int(''.join(token.value for token in items))

def hour(self, items):
return int(''.join(token.value for token in items))

def minute(self, items):
return int(''.join(token.value for token in items))

def second(self, items):
return ''.join(token.value for token in items)

def millisecond(self, items):
if items:
return ''.join(token.value for token in items)
else:
return None

def datetime_value(self, items):

year, month, day, hour, minute, second, millisecond = items

return {
"year": year,
"month": month,
"day": day,
"hour": hour,
"minute": minute,
"second": float(second + f".{millisecond if millisecond else ''}")
}
class DateError:
pass

class DateErrorParsing(DateError):
def __init__(self):
self.type = "syntax"
self.msg = "(syntax error)"

class DateErrorContent(DateError):
def __init__(self):
self.type = "content"
self.msg = "(content error)"

def parse_datetime(datetime_value):
try:
tree = parser.parse(datetime_value)
transformer = DatetimeTransformer()
parsed_datetime = transformer.transform(tree)
return {"status": 1, "msg": parsed_datetime}
except Exception:
return DateErrorParsing()


def validate_datetime(parsed_datetime):
if parsed_datetime["year"] == 0:
yield DateErrorContent()

# Month
if not (parsed_datetime["month"] > 0 and parsed_datetime["month"] < 13):
yield DateErrorContent()

# Day
if not (parsed_datetime["day"] > 0 and parsed_datetime["day"] < 32):
yield DateErrorContent()

# Check February and Leap year case
if parsed_datetime["month"] == 2:
if int(parsed_datetime["year"]) % 4 == 0:
if not (parsed_datetime["day"] < 30):
yield DateErrorContent()
else:
if not (parsed_datetime["day"] < 29):
yield DateErrorContent()

# Hour
if not (parsed_datetime["hour"] > -1 and parsed_datetime["hour"] < 24):
yield DateErrorContent()
# Minute
if not (parsed_datetime["minute"] > -1 and parsed_datetime["minute"] < 60):
yield DateErrorContent()
# Second
if not (parsed_datetime["second"] > -1 and parsed_datetime["second"] < 60):
yield DateErrorContent()
105 changes: 103 additions & 2 deletions src/ifcopenshell-python/ifcopenshell/validate.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import ifcopenshell
import ifcopenshell.express.rule_executor

from type_checker import parse_datetime, validate_datetime, DateError, DateErrorContent, DateErrorParsing

named_type = ifcopenshell.ifcopenshell_wrapper.named_type
aggregation_type = ifcopenshell.ifcopenshell_wrapper.aggregation_type
simple_type = ifcopenshell.ifcopenshell_wrapper.simple_type
Expand Down Expand Up @@ -271,6 +273,51 @@ def get_entity_attributes(schema, entity):
return entity_attrs


def check(value, type, instance):
if value is None:
return

# @nb something can be a named type with rules and still be an aggregation.
# case in point IfcCompoundPlaneAngleMeasure. Therefore only unpack named
# type references from this point onwards.
while isinstance(
type,
(
ifcopenshell.ifcopenshell_wrapper.named_type,
ifcopenshell.ifcopenshell_wrapper.type_declaration,
),
):
type = type.declared_type()

if isinstance(value, (list, tuple)):
if isinstance(type, ifcopenshell.ifcopenshell_wrapper.aggregation_type):
ty = type.type_of_element()
for v in value:
res = yield from check(v, ty, instance=instance)
if res:
yield res
else:
# Let's hope a schema validation error was reported for this case
pass

elif isinstance(value, ifcopenshell.entity_instance):
if isinstance(
value.is_a(),
ifcopenshell.ifcopenshell_wrapper.entity,
):
# top level entity instances will be checked on their own
pass
else:
# unpack the type instance
yield from check(value[0], value.is_a(), instance=instance)

if value.is_a() == "IfcDateTime" and value[0]:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function still does the type check here, so can't be reused.

Rather yield the values and do the check outside of the function. That way the same function can also be used for checking uniqueness for example.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it safe to always use value[0]? We need to add some test files where the IfcDateTime is used as an actual attribute such as IfcLibraryInformation and not just a simple type in a select, such as IfcPropertyListValue.

parsing = parse_datetime(value[0])
if isinstance(parsing, DateError().__class__):
yield DateErrorParsing()
else:
yield parsing

def validate(f, logger, express_rules=False):
"""
For an IFC population model `f` (or filepath to such a file) validate whether the entity attribute values are correctly supplied. As this
Expand Down Expand Up @@ -365,7 +412,61 @@ def validate(f, logger, express_rules=False):

if not has_invalid_value:
for i, (attr, val, is_derived) in enumerate(zip(attrs, values, entity.derived())):

try:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is a try catch statement used here? Catching all subtypes of Exception silences a lot of other issues. Please rewrite to proper if statement.

type_of_attribute = attr.type_of_attribute()
declared_type = type_of_attribute.declared_type()
type_name = declared_type.name()

if type_name == "IfcDateTime" and val:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would implement validation of https://ifc43-docs.standards.buildingsmart.org/IFC/RELEASE/IFC4x3/HTML/lexical/IfcDuration.htm in this PR as well. And really make sure that the code is as general and reusable as possible. So that validators are stored for example in a global mapping like:

type_validators = {
  'IfcDateTime: [parse_datetime, validate_datetime],
  'IfcDuration: [ ... ]
  ...
}


parsing = parse_datetime(val)

if isinstance(parsing, DateErrorParsing().__class__):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if isinstance(parsing, DateErrorParsing().__class__):
if isinstance(parsing, DateErrorParsing):

DateErrorParsing().class == DateErrorParsing

logger.error(
"For instance:\n %s\n %s\nInvalid attribute value %s for %s.%s",
inst,
annotate_inst_attr_pos(inst, i),
parsing.msg,
entity,
attrs[i],
)
elif isinstance(validate_datetime(parsing), DateErrorContent().__class__):
logger.error(
"For instance:\n %s\n %s\nInvalid attribute value %s for %s.%s",
inst,
annotate_inst_attr_pos(inst, i),
parsing.msg,
entity,
attrs[i],
)
Comment on lines +425 to +441
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these two branches are equal?


except Exception as e:
is_aggreg = isinstance(type_of_attribute, ifcopenshell.ifcopenshell_wrapper.aggregation_type)

if is_aggreg:
recursed_result = check(val, type_of_attribute, instance=inst)
for r in recursed_result:
if isinstance(r,DateErrorParsing().__class__):
logger.error(
"For instance:\n %s\n %s\nInvalid attribute value %s for %s.%s",
inst,
annotate_inst_attr_pos(inst, i),
r.msg,
entity,
attrs[i],
)
else:
other_check = list(validate_datetime(r["msg"]))
if other_check and isinstance(other_check[0], DateErrorContent().__class__):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only other_check[0]?

logger.error(
"For instance:\n %s\n %s\nInvalid attribute value %s for %s.%s",
inst,
annotate_inst_attr_pos(inst, i),
other_check[0].msg,
entity,
attrs[i],
)

if is_derived and not isinstance(val, ifcopenshell.ifcopenshell_wrapper.attribute_value_derived):
if hasattr(logger, "set_state"):
logger.set_state('attribute', f"{entity.name()}.{attr.name()}")
Expand Down Expand Up @@ -471,4 +572,4 @@ def conv(x):
return x.get_info(scalar_only=True)
else:
return str(x)
print("\n".join(json.dumps(x, default=conv) for x in logger.statements))
print("\n".join(json.dumps(x, default=conv) for x in logger.statements))
10 changes: 10 additions & 0 deletions src/ifcopenshell-python/test/fixtures/validate/fail-datetimes.ifc
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
ISO-10303-21;
HEADER;
FILE_DESCRIPTION(('ViewDefinition [CoordinationView]'),'2;1');
FILE_NAME('','2022-10-01T16:31:47',(),(),'IfcOpenShell 0.7.0','IfcOpenShell 0.7.0','');
FILE_SCHEMA(('IFC4'));
ENDSEC;
DATA;
#5=IFCPROPERTYLISTVALUE('0axfTuaRz8Zg2iRvSksWX_',$,(IFCDATETIME('2008-12-27_18:00:04'),IFCDATETIME('-208-12-27T15:00:04.02'),IFCDATETIME('80-8-03T00:00:00'),IFCDATETIME('0000-08-03T00:00:00'),IFCDATETIME('-2018-26-12T15:04:02')),$)
ENDSEC;
END-ISO-10303-21;
10 changes: 10 additions & 0 deletions src/ifcopenshell-python/test/fixtures/validate/pass-datetimes.ifc
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
ISO-10303-21;
HEADER;
FILE_DESCRIPTION(('ViewDefinition [CoordinationView]'),'2;1');
FILE_NAME('','2022-10-01T16:31:47',(),(),'IfcOpenShell 0.7.0','IfcOpenShell 0.7.0','');
FILE_SCHEMA(('IFC4'));
ENDSEC;
DATA;
#5=IFCPROPERTYLISTVALUE('0axfTuaRz8Zg2iRvSksWX_',$,(IFCDATETIME('2018-12-27T15:00:04.000054'),IFCDATETIME('-2018-12-27T15:00:04.02'),IFCDATETIME('0480-08-03T00:00:00'),IFCDATETIME('-12000-01-02T00:00:00'),IFCDATETIME('-0480-08-03T00:00:00')),$)
ENDSEC;
END-ISO-10303-21;