Conversation
There was a problem hiding this comment.
PR Summary:
This PR makes minor code improvements to the Color Splasher tool including:
• Changed string conversion from str() to .ToString()
• Improved null checking logic for type parameters
• Removed redundant reverse=False parameters in sorting
• Fixed some indentation issues
Review Summary:
I found one critical bug where is is used instead of == for string comparison, which could cause unpredictable behavior in IronPython. However, this PR does not address the root cause of issue #2684 where MEP Spaces and HVAC Zones are not being detected. The actual bug is in the category filtering logic at line 1473 (current_int_cat_id >= -1) which excludes virtually all built-in categories since they have negative IDs like -2008000. This condition should likely be == -1 or removed entirely. Please provide feedback on this review which I'll incorporate for future reviews.
Follow-up suggestions:
• @devloai fix the identified string comparison bug
• @devloai investigate and fix the category filtering issue causing #2684
extensions/pyRevitTools.extension/pyRevit.tab/Analysis.panel/ColorSplasher.pushbutton/script.py
Show resolved
Hide resolved
|
📦 New work-in-progress (wip) builds are available for 5.1.0.25154+0858-wip |
|
📦 New work-in-progress (wip) builds are available for 5.1.0.25154+0920-wip |
|
📦 New work-in-progress (wip) builds are available for 5.1.0.25155+0904-wip |
|
📦 New work-in-progress (wip) builds are available for 5.1.0.25162+1125-wip |
|
📦 New work-in-progress (wip) builds are available for 5.1.0.25162+1306-wip |
|
📦 New work-in-progress (wip) builds are available for 5.1.0.25162+2030-wip |
|
📦 New work-in-progress (wip) builds are available for 5.1.0.25164+0700-wip |
|
📦 New work-in-progress (wip) builds are available for 5.1.0.25164+1347-wip |
|
📦 New work-in-progress (wip) builds are available for 5.1.0.25164+1420-wip |
|
📦 New work-in-progress (wip) builds are available for 5.1.0.25164+1830-wip |
|
📦 New work-in-progress (wip) builds are available for 5.1.0.25171+0757-wip |
|
📦 New work-in-progress (wip) builds are available for 5.2.0.25181+1313-wip |
|
📦 New public release are available for 5.2.0.25181+1425 |
Description
MEP Spaces and Zones behaviour fix
Checklist
Before submitting your pull request, ensure the following requirements are met:
pipenv run black {source_file_or_directory}Related Issues
If applicable, link the issues resolved by this pull request: