-
-
Notifications
You must be signed in to change notification settings - Fork 85
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
Add Raith MBMS Paths #255
Add Raith MBMS Paths #255
Conversation
Thanks @MatthewMckee4 ! That's quite the effort. I'll try to go over your changes this weekend! |
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.
This is a great implementation, thanks @MatthewMckee4 for sticking to the source standards! I've added a few comments throughout the PR, if you could take a look.
Also, I'm assuming you've tested this implementation with the Raith software, right? I have no way of testing this myself except for bare minimum correctness.
Thank you, this is my first time working with c++ tbh so I'm glad to hear it's not horrible. I will get these changes updated asap. |
…nit, reordered variables in header file, fix memory leak with base_cell_name, rename PXXDATA to PXXData
@heitzmann is there any chance of this getting merged soon? I'm currently under some pressure at work to get this done so if you have a timeline that would be greatly appreciated. |
I saw a few pushes so I was not sure you were finished with the changes. If you are, we can merge it already! |
Yeah, I'm all done with my changes now. Thanks. |
Tests are failing with a segfault. Have you tested locally, @MatthewMckee4 ? |
I was having some issues so I didn't manage to test much, my apologies. I will get these issues fixed asap |
@heitzmann I am unable to reproduce the issues on my device but from the logs, I can see some issues: |
@MatthewMckee4 I went over all the changes and made a few modifications I thought necessary. Could you please test commit 2c963ba to make sure it works? I don't have a way to test the Raith data locally. If all's working, we can create a new release with the new feature. And thanks again for the push! |
@heitzmann this passes all of our tests. Should be good to merge! |
from #252. I have implemented mbms path functionality into the FlexPath Object (raith_data and base_cell_name), and added support for reading and writing to and from gds. I have also added more appropriate type hints to the gdstk.pyi file.
These changes should not affect any pre-existing functionality.