Skip to content
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

tool: Update cad_audit_check.py #2548

Merged
merged 6 commits into from
Feb 3, 2025
Merged

Conversation

nasmovk
Copy link
Contributor

@nasmovk nasmovk commented Jan 31, 2025

Update cad_audit_check.py

Description

Fixed bug in WIP 5.0.0.25025
This follows my testing of the first implementation of my cad_audit_check into pyRevit, based on my original Hackathon submittal

  1. Removed from pyrevit.forms import alert (not needed)

  2. Added class ButtonClass(Window), to manage my cancel button

  3. Added a condition to the get_load_stat(cad, is_link) function
    if status == "Loaded":
    return "Loaded"


Tested in Revit 2024 & 2025

Fixed bug in WIP 5.0.0.25025

 4. Removed from pyrevit.forms import alert (not needed)
26. Added class ButtonClass(Window), to manage my cancel button 

98. Added a condition to the get_load_stat(cad, is_link) function
    if status == "Loaded":
        return "Loaded"
Copy link
Contributor

devloai bot commented Jan 31, 2025

Automatic code review is enabled for this repository. Reviewing this PR now.

Copy link
Contributor

@devloai devloai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've reviewed the changes to cad_audit_check.py. The core improvements are good - adding proper "Loaded" status handling and improving button management with ButtonClass. I've suggested two minor improvements: removing a redundant function and maintaining consistency in status return formatting. Overall the changes are safe and well tested.

window.close()
script.exit()


# Manage Flexform cancel using .NET System.Windows RoutedEventArgs Class
def cancel_clicked(sender, e):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cancel_clicked function is now redundant since it's been moved to the ButtonClass class. Please remove this duplicate function since it's no longer used (the FlexForm now uses ButtonClass.cancel_clicked).

Co-authored-by: devloai[bot] <168258904+devloai[bot]@users.noreply.github.com>
@jmcouffin
Copy link
Contributor

@nasmovk can you check devlo's comment. After that I will merge

@jmcouffin
Copy link
Contributor

I am about to push the release button and would like to include this one in the release.

@sanzoghenzo
Copy link
Contributor

I would to the opposite as it has been done here, there's shouldn't be the need to create a class with a static method... just remove ButtonClass. form this line

Button("CANCEL", on_click=ButtonClass.cancel_clicked),

and remove ButtonClass altogether.

@nasmovk
Copy link
Contributor Author

nasmovk commented Jan 31, 2025

I will check and update, if I can understand Andrea's remark.

@sanzoghenzo
Copy link
Contributor

I will check and update, if I can understand Andrea's remark.

Sorry if I wasn't clear enough, I assumed that, since you put a class and a static method in the code, you knew the teminology 😅

  1. remove rows 26 through 34
  2. line 78 should become Button("CANCEL", on_click=cancel_clicked),

I thought I already reviewed that, but it may be lost in the merge frenzy of the model checker competition.

The StackOverflow answer you're referring to in the comments is overcomplicating a simpler solution: the OP had to just remove the parenthesis to make things work

Button('Proceed', on_click=proceed_pressed)

instead of

Button('Proceed', on_click=proceed_pressed())

The latter calls the function before passing it to the button, while the former passes the function itself to the button, so that it is called when the button is pressed.

@jmcouffin jmcouffin changed the title Update cad_audit_check.py tool: Update cad_audit_check.py Feb 1, 2025
@jmcouffin
Copy link
Contributor

@nasmovk
Waiting for your last piece (+ another test on my side) to press the release button ;p #nopressure

@nasmovk
Copy link
Contributor Author

nasmovk commented Feb 1, 2025

I'm away skiing for the weekend, but I promise to look at this as soon as I'm back. Looking forward to the v5 launch.

@jmcouffin jmcouffin merged commit 06e079d into pyrevitlabs:develop Feb 3, 2025
Copy link
Contributor

github-actions bot commented Feb 3, 2025

📦 New work-in-progress (wip) builds are available for 5.0.0.25034+1241-wip

Copy link
Contributor

github-actions bot commented Feb 3, 2025

📦 New work-in-progress (wip) builds are available for 5.0.0.25034+1511-wip

Copy link
Contributor

github-actions bot commented Feb 3, 2025

📦 New public release are available for 5.0.0.24174+2300

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants