Skip to content

Refactor cadnano import/export#193

Merged
UnHumbleBen merged 14 commits intodevfrom
refactor-cadnano-export
Oct 9, 2021
Merged

Refactor cadnano import/export#193
UnHumbleBen merged 14 commits intodevfrom
refactor-cadnano-export

Conversation

@UnHumbleBen
Copy link
Copy Markdown
Collaborator

Description

The two main changes are the cadnano export functions and unit testing

Cadnano export refactor

The cadnano export functions has been refactored

Before

  • Design.to_cadnano_v2
  • export_cadnano_v2

After

  • Design.to_cadnano_v2_serializable
  • Design.to_cadnano_v2_json
  • write_cadnano_v2_file

in order to better align with the existing APIs for scadnano and oxdna file exports.

Unit tests no longer output files

Calls to write_scadnano_file and write_cadnano_v2_file (previously export_cadnano_v2) have been commented out.

Calls to write_cadnano_v2_file (previously export_cadnano_v2) have been replaced by invoking Design.to_cadnano_v2_json function.

Motivation and Context

This change was needed in order to make the APIs for the cadnano export consistent with the scadnano and oxdna file exports. In addition, the unit testing has been cleaned up so that they no longer write files, which is unnecessary.

How Has This Been Tested?

Ran unit tests. Also uncommented the write_cadnano_v2_file and verified that the outputted cadnano files can be opened on cadnano and that the design matches what is expected based on the scadnano design. Also verified that inputted cadnano files are properly exported to scadnano.

@UnHumbleBen UnHumbleBen requested a review from dave-doty as a code owner October 4, 2021 05:08
@UnHumbleBen UnHumbleBen requested a review from dave-doty October 8, 2021 07:06
Copy link
Copy Markdown
Member

@dave-doty dave-doty left a comment

Choose a reason for hiding this comment

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

Looks good, although in the diff (the "File changed" tab on GitHub) I don't see the new unit tests anymore. Are the new unit tests there? Maybe I'm only looking at a diff between the last two commits, rather than a diff between the dev branch and this one.

@UnHumbleBen
Copy link
Copy Markdown
Collaborator Author

UnHumbleBen commented Oct 8, 2021

Looks good, although in the diff (the "File changed" tab on GitHub) I don't see the new unit tests anymore. Are the new unit tests there? Maybe I'm only looking at a diff between the last two commits, rather than a diff between the dev branch and this one.

Are you looking for these?

New unit tests for import:
https://github.com/UC-Davis-molecular-computing/scadnano-python-package/pull/193/files#diff-06647c09ad5a213a9c27accba8394e109a9a570444af08a1b38ea5b50ca4e7dcR587-R712

New unit tests for export:
https://github.com/UC-Davis-molecular-computing/scadnano-python-package/pull/193/files#diff-06647c09ad5a213a9c27accba8394e109a9a570444af08a1b38ea5b50ca4e7dcL639

You might need to click on Load Diff in test/scadnano_test.py to see it

Copy link
Copy Markdown
Member

@dave-doty dave-doty left a comment

Choose a reason for hiding this comment

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

Ah, it was hidden by the "large diffs not rendered by default".

@UnHumbleBen UnHumbleBen merged commit 7b4b82a into dev Oct 9, 2021
@UnHumbleBen UnHumbleBen deleted the refactor-cadnano-export branch October 9, 2021 00:55
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.

2 participants