-
Notifications
You must be signed in to change notification settings - Fork 0
Created fix for handling large jumps in plotted data. Reworked patch … #5
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
base: main
Are you sure you want to change the base?
Conversation
jommme
commented
Dec 10, 2024
- Created fix for handling large jumps in plotted data.
- Reworked patch collection.
- Added draw quad mesh but with no handling.
…collection. Added draw quad mesh but with no handling.
Co-authored-by: Ingeborg Gjerde <[email protected]>
mpldxf/backend_dxf.py
Outdated
| intersection = line.intersection(cliprect) | ||
| except: | ||
| intersection = Polygon() | ||
| except Exception: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we catch a more specific Exception here? Generally, it is considered bad practice to have try/except without specifying the exact exception you are expecting.
| # Behandle PolyCollection som en samling av 'patch'-objekter | ||
| for path in paths: | ||
| # Kombiner master_transform med path_transform for hver path | ||
| for i, path in enumerate(paths): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Has this been reverted to an old version of the code?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I tried to figure out where the failure happend, I tried some different approaches and that we didnt handle all_transforms was one thing I was looking into.
I just revert it for now, even though I think the new code is working asmit should as well :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I figured out it actually fixed a bug as well!
I completely forgot, but some lines from the simple sounding plot was not plotted before this change.
So I will actually leave it as it is here :)
mpldxf/backend_dxf.py
Outdated
| intersection = Polygon() | ||
| except Exception: | ||
| # Fix plotted values with large gaps/jumps crashing the intersection function | ||
| gap_threshold = 50 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's a good idea to use a hard coded value here. If we have plots ranging between 0 and 2000, for example the pore pressure in a CPT, a gap threshold of 50 is quite small.
mpldxf/backend_dxf.py
Outdated
| # Fix plotted values with large gaps/jumps crashing the intersection function | ||
| gap_threshold = 50 | ||
| smoothed_vertices = [vertices[0]] # Start with the first point | ||
| for i in range(1, len(vertices)): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use np.diff to compute this more effectively, and filter using np.where.
mpldxf/backend_dxf.py
Outdated
| def points_to_pixels(self, points): | ||
| return points / 72.0 * self.dpi | ||
|
|
||
| def draw_quad_mesh( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for this change? Do we need it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I tried to figure out where the failure happend, I tried some different approaches and figured out that we didnt handle quad mesh, so I set up some of the entities we didnt handle to see what wass passed to them.
Thought we just could keep them to be aware of them and implement them if necessary at a later stage.
But I will just remove it for now, since not in use.