-
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
chore(revit): adds units to object params #280
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #280 +/- ##
========================================
- Coverage 8.43% 8.41% -0.02%
========================================
Files 231 231
Lines 4567 4573 +6
Branches 562 563 +1
========================================
Hits 385 385
- Misses 4165 4171 +6
Partials 17 17 ☔ View full report in Codecov by Sentry. |
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.
Code-wise ok to me, but can't say (yet) anything about expected behavior for this.
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.
@clairekuang could we do a very fast performance benchmark on this before we merge?
basically delete the data.db from appdata/speckle, and send the advanced sample model (should be enough) with and without units on each parameter.
it should be fine overall, but i'd want to quantify what we're sacrificing in terms of performance!
code wise everything ok!
Tested on the sample structural model, adding units was about a 5.6% increase in size :0 |
Pulls the units from the parameter definition to add to object parameters. If no units are present, there will be no unit field on the object param.
https://latest.speckle.systems/projects/3f895e614f/models/7367fe3688@03e237b76d