Update radar_check.py #2419
Update radar_check.py #2419
Conversation
jmcouffin
added
the
Enhancement
|
@tay0thman I reviewed more seriously your code, did quite a few edit for clarity and pep8 compliance
ref: #2418 |
|
Good insights, I just learned that PEP8 is a thing, one more item to learn about. I'll make some updates on my side and will generate a pull request early next week. Thank you @jmcouffin for your time reviewing and providing feedback. |
There was a problem hiding this comment.
@tay0thman, you need to check
- code formatting, lots of missing and extra spaces,
- non-pep8 variable naming,
- the length of the lines should not be more around 79 symbols
- shadows names
| # Create a 3D view | ||
| t = Transaction(doc, "Create 3D View") |
There was a problem hiding this comment.
I recommend using with statement for any transaction, if is possible better use revit.Transaction class
| violatingRVT = [] | ||
| badelements = [] | ||
| def get_tempbbox(self, toggle_CAD, toggle_RVT, toggle_IFC, toggle_all): | ||
| violating_CAD = [] |
There was a problem hiding this comment.
Variable in function should be lowercase
| class Get3DViewBoundingBox(): | ||
| def get_tempbbox(self, ToggleCAD, ToggleRVT, ToggleIFC, ToggleAll): |
There was a problem hiding this comment.
Argument name should be lowercase
| view3dtype = [v for v in view3Dtypes if v.ViewFamily == DB.ViewFamily.ThreeDimensional][0] | ||
| view = DB.View3D.CreateIsometric(doc, view3dtype.Id) | ||
| worksets = FilteredWorksetCollector(doc).OfKind(WorksetKind.UserWorkset).ToWorksets() | ||
| view_3D_types = DB.FilteredElementCollector(doc).OfClass(DB.ViewFamilyType).WhereElementIsElementType().ToElements() |
There was a problem hiding this comment.
PEP 8: E501 line too long (124 > 120 characters)
| value=distance, | ||
| maxAccuracy=False, | ||
| forEditing=False) | ||
| ui_units = DB.UnitFormatUtils.Format(units=doc.GetUnits(), unitType=DB.UnitType.UT_Length, value=distance, maxAccuracy=False, forEditing=False) |
There was a problem hiding this comment.
It is advisable to make the code formatting similar in the same places
| @@ -266,20 +241,19 @@ def check_model_extents(doc, output): | |||
| print(output.linkify(y.Id)+ str(y.Name)+ " - Is part of design option - "+ str(y.DesignOption.Name) ) | |||
There was a problem hiding this comment.
missing spaces, formatting is messed up
| for element in elements: | ||
| if element.get_BoundingBox(view) is not None and hasattr(element, 'Name') and hasattr(element, 'Category'): | ||
| bbox = element.get_BoundingBox(view) | ||
| if check_bounding_box(bbox, INTERNAL_ORIGIN, 52800) == 0: |
There was a problem hiding this comment.
magic number, needs create constant or comment with unit of measurement
| #__________________________________________check the distnaces of base and survey points | ||
| divider = "_"*100 | ||
| test_score = 0 | ||
| # __________________________________________check the distnaces of base and survey points |
|
|
||
| output.print_md('### :OK_hand_medium_skin_tone: ............Survey Point is less than 10 miles (16KM) away from the Internal Origin.') | ||
| baseptdistance = calculate_distance(basept, INTERNAL_ORIGIN) | ||
| tabledata = [['InternaL Origin Coordinates', str(INTERNAL_ORIGIN)], |
| output.print_md('### :thumbs_down_medium_skin_tone: ............Distant objects are still being detected!') | ||
| output.print_md('### :warning: ............Further Analysis Required.') | ||
| else: | ||
| output.print_md('### :OK_hand_medium_skin_tone: ............All Objects are located less than 10 miles (16KM) away from the Internal Origin.***') | ||
| output.print_md('### :OK_hand_medium_skin_tone: ............All Objects are located less than 10 miles (16KM) away from the Internal Origin.') | ||
| sys.exit() |
There was a problem hiding this comment.
I think better to use script.exit() docs
|
Thank you @dosymep for your thorough review and feedback, good to know how important these items are and will change the code to reflect these recommendations very shortly. Thanks again |
reviewed tayO radar check