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

Improve AECVision integration panel #12

Open
PawelKinczyk opened this issue Aug 6, 2023 · 0 comments
Open

Improve AECVision integration panel #12

PawelKinczyk opened this issue Aug 6, 2023 · 0 comments
Labels
improvement Something that can improve the project

Comments

@PawelKinczyk
Copy link
Owner

In relation to the post: https://discourse.pyrevitlabs.io/t/walls-shifts-while-create-from-model-detection-csv/1951/3
sanzoghenzo suggested some changes:

Hi @PanPawel_1, great work here!

I didn’t test your code, put I see you’re printing point_1 and point_2; are their coordinates the expected ones? You can manually calculate them using some excel formula and compare the results.
If they are, then there’s something wrong in the wall_line or the Wall.Create method.

That being said, I can suggest some modifications in the code:

-     pyrevit.forms function don’t raise exception on no selection, they simply return an empty string, so you should check the value of the variable instead of use try/except;
-     you could read the csv and transform the numbers in one go: data_file = [float_values(row) for row in data]
-     don’t use dict as variable name, is a reserved word for the dictionary class
-     SelectFromList can accept a list of object and get the text to display with the name_attr argument, so you could use selected_walls = forms.SelectFromList.show(walls, name_attr="Name", ...) and directly get the list of selected walls (again, don’t catch exceptions but simply check if the list is empty with if not selected_walls:). The same applies to levels;
-     Instead of creating the walls_list list of tuples and then separate it, just create wall_thickness = [w.Width for w in selected_walls]. Or you can even drop that list and directly get the matching wall with min(selected_walls, key=lambda w: abs(w.Width - thickness));
-     It’s always best to catch a specific exception to avoid swallowing unpredictable errors; after cleaning up the selections, there’s only the float() function that can error, and it can throw ValueError or TypeError.

The following are my preferences:

-     the DB namespace/module can also be accessed by from pyrevit.revit import DB (pyrevit already takes care of loading Revit assemblies);
-     the doc variable can also be accessed by pyrevit.revit.doc;
-     the Transaction object you get by from pyrevit.revit import Transaction takes care of handling the commit/rollback via a context manager: use with Transaction(): and indent all the operations that needs to be done inside it. it also shows a message in case of errors, so no need to try/except;
-     instead of keep calling dict[key] I would use temporary variables to store xmin, xmax, ymin and ymax, and also use other temporary variables to store the x_delta and y_delta instead of repeating the calculation.
@PawelKinczyk PawelKinczyk added the improvement Something that can improve the project label Aug 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Something that can improve the project
Projects
None yet
Development

No branches or pull requests

1 participant