-
-
Notifications
You must be signed in to change notification settings - Fork 677
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
base: v0.7.0
Are you sure you want to change the base?
Changes from all commits
f0d4691
4a2f34a
5c5aaf1
9021d12
0f75565
73834ea
d0abe8f
6b57a57
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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() |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -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 | ||||||
|
@@ -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]: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is it safe to always use |
||||||
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 | ||||||
|
@@ -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: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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: | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
|
||||||
|
||||||
parsing = parse_datetime(val) | ||||||
|
||||||
if isinstance(parsing, DateErrorParsing().__class__): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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__): | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why only |
||||||
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()}") | ||||||
|
@@ -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)) |
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; |
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; |
There was a problem hiding this comment.
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.