-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Assembly: migrationScript2 and migrationScript4 improved and refactored #18100
base: main
Are you sure you want to change the base?
Conversation
Description of Defects in migrationScript2 and migrationScript4: Several additional issues have been identified in the Assembly functions migrationScript4 and migrationScript2, beyond those fixed in patch 71f0104. One common problem is that the Joint property Reference1 or Reference2 can be set to an invalid format. For example, user can manually cause this issue by assigning an incorrect reference (e.g., the Z-axis) to these properties via the FreeCAD GUI. When this happens, internal errors (exceptions and their corresponding Python call stack) are reported in the Report View. However, such information is not helpful for the user to fix their 3D model. Issues with the Current Implementation (Using migrationScript4 as Reference): - Lack of Type Validation for Reference1 and Reference2: The script does not check whether joint.Reference1[1] or joint.Reference2[1] is a sequence. If this condition is not met, statements like: sub1 = joint.Reference1[1][0] will throw an exception, printing the Python call stack. - Lack of Length Validation for Sequences: The script does not ensure that the sequence length is sufficient for indexing. For example, joint.Reference1 and joint.Reference2 must each have at least two elements to allow: joint.Reference1[1][1] joint.Reference2[1][1] Without validation, such calls can result in exceptions, printing the Python call stack. - No Feedback on Problematic Joint: The user is not informed about which Assembly joint in the object hierarchy has an invalid Reference1 or Reference2. Depending on the size of the 3D model, identifying the defect can be time-consuming and frustrating without proper feedback. This patch is focused on: - Reporting Problematic Joint Path: A new function has been added to retrieve the full path to the Assembly joint. The return value (a string) is used to report the issue. - Improved Exception Handling: Instead of adding more conditional checks (see above), exception handling is implemented. AttributeError, IndexError, and TypeError exceptions are caught. The exception message (excluding the call stack) is displayed at the end of a user-friendly warning. - User-Friendly Warnings in the Report View: A single-line warning is now shown in the Report View, containing the following details: - Source of Warning: 'Assembly joint' - Location in the 3D Model: e.g., 'Assembly_XY.object_XY.joint_XY' - Problematic Attribute: e.g., 'ReferenceXY' - Exception Message: e.g., 'list index out of range' - Refactoring and Optimization: The functions were refactored and optimized at the end of the conversion process. Advantages: - Cleaner Report View: Internal errors (e.g., Python call stacks) are no longer logged in the Report View, as they are not useful for end users. Only the exception message is displayed. - Improved User Feedback: Users can now identify the problematic Assembly joint in the object hierarchy, saving significant time and effort. - Robust and Concise Code: The solution simplifies the code while increasing its robustness.
Looks good to me. @Jiri-Macha I supposed you have tested the migration scripts with various type of old files (and new files as well) just to make sure? |
@PaddleStroke The most complex FreeCAD model I have made so far is composed of approximately 23 files. As a FreeCAD user, I would also appreciate having the document name displayed before the So, to answer your question: yes, I tested it, but I didn't perform extensive I do not have many assemblies created in the new Assembly workbench because it is still too new. I didn't change the logic of the migration code; I only improved its behavior when there is a wrong |
Description of Defects in migrationScript2 and migrationScript4:
Several additional issues have been identified in the Assembly functions migrationScript4 and migrationScript2, beyond those fixed in patch 71f0104.
One common problem is that the Joint property Reference1 or Reference2 can be set to an invalid format. For example, user can manually cause this issue by assigning an incorrect reference (e.g., the Z-axis) to these properties via the FreeCAD GUI. When this happens, internal errors (exceptions and their corresponding Python call stack) are reported in the Report View. However, such information is not helpful for the user to fix their 3D model.
Issues with the Current Implementation (Using migrationScript4 as Reference):
Lack of Type Validation for Reference1 and Reference2:
The script does not check whether joint.Reference1[1] or joint.Reference2[1] is a sequence. If this condition is not met, statements like:
sub1 = joint.Reference1[1][0]
will throw an exception, printing the Python call stack.
Lack of Length Validation for Sequences:
The script does not ensure that the sequence length is sufficient for indexing. For example, joint.Reference1 and joint.Reference2 must each have at least two elements to allow:
joint.Reference1[1][1]
joint.Reference2[1][1]
Without validation, such calls can result in exceptions, printing the Python call stack.
No Feedback on Problematic Joint:
The user is not informed about which Assembly joint in the object hierarchy has an invalid Reference1 or Reference2. Depending on the size of the 3D model, identifying the defect can be time-consuming and frustrating without proper feedback.
This patch is focused on:
Reporting Problematic Joint Path:
A new function has been added to retrieve the full path to the Assembly joint. The return value (a string) is used to report the issue.
Improved Exception Handling:
Instead of adding more conditional checks (see above), exception handling is implemented. AttributeError, IndexError, and TypeError exceptions are caught. The exception message (excluding the call stack) is displayed at the end of a user-friendly warning.
User-Friendly Warnings in the Report View:
A single-line warning is now shown in the Report View, containing the following details:
Refactoring and Optimization:
The functions were refactored and optimized at the end of the conversion process.
Advantages:
Cleaner Report View: Internal errors (e.g., Python call stacks) are no longer logged in the Report View, as they are not useful for end users. Only the exception message is displayed.
Improved User Feedback: Users can now identify the problematic Assembly joint in the object hierarchy, saving significant time and effort.
Robust and Concise Code: The solution simplifies the code while increasing its robustness.