Skip to content

Dev export helix groups#133

Merged
dave-doty merged 16 commits intodevfrom
dev-export-helix-groups
Sep 10, 2020
Merged

Dev export helix groups#133
dave-doty merged 16 commits intodevfrom
dev-export-helix-groups

Conversation

@tcosmo
Copy link
Copy Markdown
Collaborator

@tcosmo tcosmo commented Sep 8, 2020

No description provided.

@tcosmo tcosmo requested a review from dave-doty as a code owner September 8, 2020 16:32
@tcosmo tcosmo linked an issue Sep 8, 2020 that may be closed by this pull request
@tcosmo
Copy link
Copy Markdown
Collaborator Author

tcosmo commented Sep 8, 2020

Designs with helix groups sharing same grid are now exportable. Also working on cadnano oligos names not done yet.

domain, next_domain, strand_type)

def _cadnano_v2_fill_blank(self, dct: dict, num_bases: int) -> Dict[int, int]:
def _cadnano_v2_fill_blank(self, dct: dict, num_bases: int, design_grid: Grid) -> dict:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did I get the type wrong here? I had annotated the return type of _cadnano_v2_fill_blank as Dict[int, int] based on looking at usages in the definition.

If that's wrong, can we specify a more specific type than dict? Or do the types of the the values and keys vary?

If only the values vary, but the keys are always int, then it's still better to use Dict[int, Any] (instead of dict)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Solved with 3753e36

@dave-doty dave-doty self-requested a review September 10, 2020 14:36
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.

I approved the PR, but then noticed that unit tests are not passing. Looking at the logs, I think there is a syntax error somewhere. Can you run the unit tests locally and see where the problem is?

@tcosmo
Copy link
Copy Markdown
Collaborator Author

tcosmo commented Sep 10, 2020

@dave-doty my very bad, solved with 627d45b

@dave-doty dave-doty merged commit 5ace679 into dev Sep 10, 2020
@dave-doty dave-doty deleted the dev-export-helix-groups branch September 10, 2020 16:01
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.

Exporting designs with helix groups to cadnano v2

2 participants