-
Notifications
You must be signed in to change notification settings - Fork 7
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
CNX-751: Add Units to reportProperties #389
CNX-751: Add Units to reportProperties #389
Conversation
…tproperties' into bjorn/cnx-751-add-units-to-reportproperties
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #389 +/- ##
=====================================
Coverage 8.37% 8.37%
=====================================
Files 237 237
Lines 4704 4704
Branches 513 513
=====================================
Hits 394 394
Misses 4294 4294
Partials 16 16 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
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.
Looks good to me! And thanks for saving me trying to understand whats up, nice screenshots! ❤️
if @dogukankaratas doesn't have any objection or catch, we can merge it.
As discussed
Description & motivation
Changes:
Tekla.Structures.Datatype
enums
for units instead ofTekla.Structures.Drawing
. This makes more sense as theDrawing
class is more geared to dimension lines, annotations etc. than the model. Here, we also get rid of theFeetsAndInches
,CentimetersOrMeters
andAutomatic
enums.hostUnit
fromTekla.Structures.Datatype.Distance.CurrentUnitType
returns the correct unit set inSettings > Options > Units and decimals
, but API returns are always mm. Potential mismatch in model units and report property BUT we append the units to each report property.reportProperties
are default mm (or power of). Hence, units appended are hard-coded. Awaiting response and assistance from Tekla. This can be done much better.Screenshots:
More explanation:
Settings > Options > Units and decimals
). But these methods only apply toDistance
andAngle
params. Probably more risky to go through series of conversions AND writing additional ones for area, volume etc. than for us to just report everything as internal units.AREA
is not the cross-sectional area of the section but some other value. Not our issue though, this is as per Tekla reportVOLUME
is, however, calculated using the true cross-sectional area multiplied by length. Weird.Validation of changes:
Basic Steel Model.db1
and checking returned values with generated report.To Do Before Merge
Checklist: