heitzmann / gdstk

Gdstk (GDSII Tool Kit) is a C++/Python library for creation and manipulation of GDSII and OASIS files.
https://heitzmann.github.io/gdstk/
Boost Software License 1.0
337 stars 84 forks source link

Add Raith MBMS Paths #255

Closed MatthewMckee4 closed 3 months ago

MatthewMckee4 commented 3 months ago

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.

heitzmann commented 3 months ago

Thanks @MatthewMckee4 ! That's quite the effort. I'll try to go over your changes this weekend!

MatthewMckee4 commented 3 months ago

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.

MatthewMckee4 commented 3 months ago

@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.

heitzmann commented 3 months ago

I saw a few pushes so I was not sure you were finished with the changes. If you are, we can merge it already!

MatthewMckee4 commented 3 months ago

Yeah, I'm all done with my changes now. Thanks.

heitzmann commented 3 months ago

Tests are failing with a segfault. Have you tested locally, @MatthewMckee4 ?

MatthewMckee4 commented 3 months ago

I was having some issues so I didn't manage to test much, my apologies. I will get these issues fixed asap

MatthewMckee4 commented 3 months ago

@heitzmann I am unable to reproduce the issues on my device but from the logs, I can see some issues: image I have fixed the issue on line 160, this was simply me writing the wrong method. The other issue though comes from: image where dwelltime_selection is: image would I be right in changing this to: image or image If not whats the correct casting function?

heitzmann commented 3 months ago

@MatthewMckee4 I went over all the changes and made a few modifications I thought necessary. Could you please test commit https://github.com/heitzmann/gdstk/commit/2c963bae25eda75b4aa1580d96cbde93cacefe8c 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!

MatthewMckee4 commented 3 months ago

@heitzmann this passes all of our tests. Should be good to merge!