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

Bug/export import existing hfss3dlayout setup #2594

Merged
merged 6 commits into from Mar 8, 2023

Conversation

isaacansys
Copy link
Collaborator

@isaacansys isaacansys commented Mar 7, 2023

Updates SetupTemplates.HFSS3DLayout and child templates to 2023 R1 and adds a test for export/import round trip from an existing HFSS 3D Layout design and setup, which would fail prior to this fix (e.g. at 5eb876d). Reasons for draft status:

  • The templates could/should be made version specific as it's not clear to me what happens if we try to update an older version setup with newer version properties; the added test passes in 2022.1 and 2022.2 even though the template now contains properties from later versions but I'm not sure this is OK to do
  • An alternative "fix" could be to simply remove the check whether each key in the read JSON data is in the setup; this would allow any version to work at the expense of missing issues with hand-edited files; the template in this case could be reduced to a minimal set of properties assuming other properties would be set to defaults by the Create and Edit setup API commands

Fixes #2593

@maxcapodi78
Copy link
Collaborator

Hi @isaacansys I think the two approaches proposed by you are not excluding each other. I'm working to a potential fix for point 1 while we could make the import_from_json more tolerant during the import phase.
What do you think?

@maxcapodi78
Copy link
Collaborator

maxcapodi78 commented Mar 8, 2023

@isaacansys please give a look to the modifications I've pushed and let me know if you agree on this approach
We just need to decide if the unexpected key should be added anyway to props (maybe a key not in the template) or should it be skipped

@codecov
Copy link

codecov bot commented Mar 8, 2023

Codecov Report

Merging #2594 (76e2078) into main (27decbe) will decrease coverage by 0.03%.
The diff coverage is 90.00%.

❗ Current head 76e2078 differs from pull request most recent head 5a8cf3d. Consider uploading reports for the commit 5a8cf3d to get more accurate results

@@            Coverage Diff             @@
##             main    #2594      +/-   ##
==========================================
- Coverage   80.16%   80.14%   -0.03%     
==========================================
  Files         164      163       -1     
  Lines       53136    53070      -66     
==========================================
- Hits        42599    42533      -66     
  Misses      10537    10537              

isaacansys and others added 6 commits March 8, 2023 15:06
Add a test for exporting and importing a setup from an existing HFSS 3D
Layout design with an existing setup.  This currently fails.
- Update SetupTemplates.HFSS3DLayout for 2022 R1
- Filter some internal-only keys during export_to_json
Add example projects and extend the test case for #2593 to be version
specific for 2022.1, 2022.2, and 2023.1.
…ort_from_json is now tolerant on wrong keys.
…ort_from_json is now tolerant on wrong keys.
Now that set_props will issue a warning instead of throwing an exception
when it encounters a new property key, if the value is a dictionary we
need to initialize the key to an empty dictionary before recursing.
@isaacansys isaacansys force-pushed the bug/export-import-existing-hfss3dlayout-setup branch from 5456530 to 5a8cf3d Compare March 8, 2023 20:06
@isaacansys isaacansys marked this pull request as ready for review March 8, 2023 20:06
Copy link
Collaborator Author

@isaacansys isaacansys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@isaacansys isaacansys enabled auto-merge (squash) March 8, 2023 20:07
@isaacansys isaacansys merged commit fe7f0a2 into main Mar 8, 2023
@isaacansys isaacansys deleted the bug/export-import-existing-hfss3dlayout-setup branch March 8, 2023 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot export_to_json followed by import_from_json with existing HFSS3DLayout setup
2 participants