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

Obsolete IntegerValue property in Revit2024 and higher - Replaced by Value #2380

Merged
merged 66 commits into from
Aug 29, 2024

Conversation

jmcouffin
Copy link
Contributor

@jmcouffin jmcouffin added this to the pyRevit 5 RC milestone Aug 27, 2024
@jmcouffin jmcouffin added the API Change Revit API change that breaks pyRevit [class->Upgraded #{number}: {title}] label Aug 27, 2024
@sanzoghenzo
Copy link
Contributor

sanzoghenzo commented Aug 27, 2024

Can I offer a suggestion to avoid code repetition?

value_func  = lambda x: x.Value if HOST_APP.is_newer_than(2023) else x.IntegerValue

or a more verbose, but maybe clearer, function to put in compat.py

def get_value_func():
    def get_value_2024(item):
        return item.Value

    def get_value_2003(item):
        return item.IntegerValue

    return get_value_2024 if HOST_APP.is_newer_than(2023) else get_value_2003

so that one can do, for example

value_func = get_value_func()
sheet_revids = {value_func(x) for x in self.revit_sheet.GetAllRevisionIds()}
add_sheet_revids = {value_func(x) for x in self.revit_sheet.GetAdditionalRevisionIds()}

@jmcouffin
Copy link
Contributor Author

Great advice @sanzoghenzo
As always.
I did not overthink the thing, I just did the mods over lunch.
I will implement your 2nd suggestion

Copy link
Contributor

@sanzoghenzo sanzoghenzo left a comment

Choose a reason for hiding this comment

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

This is not properly in-scope, but while you're at it.... 😉

pyrevitlib/rpw/db/family.py Outdated Show resolved Hide resolved
@sanzoghenzo
Copy link
Contributor

Just a question: why there were lines like if cat.Id.IntegerValue == cat_id.IntegerValue?

I'm not a Revit API guru, but I expect that cat.Id == cat_id would do just fine and save some CPU time in the python side of things...

Obviously I don't have the full picture, and I didn't check it, but I can see the Equals operator for ElementId in revitapidocs

@jmcouffin jmcouffin marked this pull request as ready for review August 28, 2024 18:05
@jmcouffin
Copy link
Contributor Author

@sanzoghenzo would you mind having a quick look.
My eyes are sore editing 46 files with dumb changes like this.
I had to stop looking and just do the thing as there are so many coding styles and bad practices in so many tools 🤣

@jmcouffin
Copy link
Contributor Author

I may have broken a thing or two. Time will tell.
A common pattern that bothers me is the ElementId comparison to -1 instead of

if isinstance(element, DB.ElementId.InvalidElementId):

Copy link
Contributor

@sanzoghenzo sanzoghenzo left a comment

Choose a reason for hiding this comment

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

You did a hell of a job!

some of the variables are enum items, so we should use the value to compare them.
I suggested only one place in which to use DB.ElementId.InvalidElementId (it's not a type so we should use comparison operators and not isinstance), but there may be other places it can be used.

pyrevitlib/pyrevit/revit/db/pickling.py Outdated Show resolved Hide resolved
pyrevitlib/pyrevit/revit/db/query.py Outdated Show resolved Hide resolved
pyrevitlib/pyrevit/revit/db/query.py Outdated Show resolved Hide resolved
pyrevitlib/pyrevit/revit/db/query.py Outdated Show resolved Hide resolved
pyrevitlib/rpw/db/builtins.py Outdated Show resolved Hide resolved
pyrevitlib/rpw/db/family.py Outdated Show resolved Hide resolved
@jmcouffin jmcouffin merged commit ce2534d into pyrevitlabs:develop Aug 29, 2024
@sanzoghenzo
Copy link
Contributor

Argh, I was going to suggest you to do a merge with squash to avoid all those commits in the git log 😅

It's getting harder and harder browse the commit history with all those single file commits....

Copy link
Contributor

📦 New work-in-progress (wip) builds are available for 5.0.0.24242+0910-wip

Copy link
Contributor

github-actions bot commented Sep 2, 2024

📦 New work-in-progress (wip) builds are available for 5.0.0.24246+1410-wip

@sanzoghenzo
Copy link
Contributor

Did you test this PR? It is broken since you used __revit__.Application.is_newer_than, but the Revit Application class doesn't have that method. You need to use HOST_APP.is_newer_than to make it work.

I'll fix it after lunch

@jmcouffin
Copy link
Contributor Author

Did you test this PR?
I haven't
I guess HOST_APP won't cut it either as it is initiated in the init, right? Inception situation?
something like __revit__.Application.VersionNumber > 2023 should work

@jmcouffin
Copy link
Contributor Author

and for the sake of documentation, this release works in 2025 but not in 2024 and below
image

@sanzoghenzo
Copy link
Contributor

That error is the one I'm trying to fix on my PR 😉

I'll factor out the version number retrieval from the is_netcore to use it for this function, too.

@jmcouffin
Copy link
Contributor Author

I'll fix it after lunch

You were not kidding! @sanzoghenzo Nice!

Copy link
Contributor

github-actions bot commented Sep 9, 2024

📦 New work-in-progress (wip) builds are available for 5.0.0.24253+1920-wip

Copy link
Contributor

📦 New work-in-progress (wip) builds are available for 5.0.0.24254+1158-wip

Copy link
Contributor

📦 New work-in-progress (wip) builds are available for 5.0.0.24254+1351-wip

Copy link
Contributor

📦 New work-in-progress (wip) builds are available for 5.0.0.24261+0647-wip

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Change Revit API change that breaks pyRevit [class->Upgraded #{number}: {title}]
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants