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

228 prevent two strands with the same name #257

Open
wants to merge 16 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
.DS_Store
.idea

output
Expand Down
63 changes: 35 additions & 28 deletions nuad/constraints.py
Original file line number Diff line number Diff line change
Expand Up @@ -1387,10 +1387,10 @@ def __eq__(self, other: Part) -> bool:
def __hash__(self) -> int:
return hash(self.key())

@property
@abstractmethod
def name(self) -> str:
pass
# @property
Copy link
Member

Choose a reason for hiding this comment

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

Let's not leave dead comments like this, just delete them.

# @abstractmethod
# def name(self) -> str:
# pass

@abstractmethod
def key(self) -> str:
Expand Down Expand Up @@ -1429,7 +1429,7 @@ class Domain(Part, JSONSerializable):
and also modifying the pool (letting the pool be assigned after it is created).
"""

_name: str
name: str
"""
Name of the :any:`Domain`.
This is the "unstarred" version of the name, and it cannot end in `*`.
Expand Down Expand Up @@ -1523,7 +1523,7 @@ def __init__(self, name: str, pool: DomainPool | None = None, sequence: str | No
subdomains: List[Domain] | None = None, weight: float | None = None) -> None:
if subdomains is None:
subdomains = []
self._name = name
self.name = name
self._starred_name = name + '*'
self._pool = pool
self._sequence = sequence
Expand Down Expand Up @@ -1573,7 +1573,7 @@ def key(self) -> str:
__hash__ = Part.__hash__

def __repr__(self) -> str:
return self._name
return self.name

def individual_parts(self) -> Tuple[Domain, ...]:
return self,
Expand Down Expand Up @@ -1630,20 +1630,20 @@ def from_json_serializable(json_map: Dict[str, Any],
domain: Domain = Domain(name=name, sequence=sequence, fixed=fixed, pool=pool, label=label)
return domain

@property
def name(self) -> str:
"""
:return: name of this :any:`Domain`
"""
return self._name

@name.setter
def name(self, new_name: str) -> None:
"""
:param new_name: new name to set
"""
self._name = new_name
self._starred_name = new_name + '*'
# @property
# def name(self) -> str:
# """
# :return: name of this :any:`Domain`
# """
# return self._name
#
# @name.setter
# def name(self, new_name: str) -> None:
# """
# :param new_name: new name to set
# """
# self._name = new_name
# self._starred_name = new_name + '*'

@property
def pool(self) -> DomainPool:
Expand Down Expand Up @@ -1850,7 +1850,7 @@ def get_name(self, starred: bool) -> str:
:return: The value :data:`Domain.name` or :data:`Domain.starred_name`, depending on
the value of parameter `starred`.
"""
return self._starred_name if starred else self._name
return self._starred_name if starred else self.name

def concrete_sequence(self, starred: bool) -> str:
"""
Expand Down Expand Up @@ -2283,7 +2283,7 @@ class Strand(Part, JSONSerializable):
or 384-well plates.
"""

_name: str | None = None
name: str
Copy link
Member

Choose a reason for hiding this comment

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

There should be an error since name is being declared after vendor_fields, since non-optional fields need to precede optional fields

"""Optional name of strand."""
Copy link
Member

Choose a reason for hiding this comment

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

name is not optional


modification_5p: nm.Modification5Prime | None = None
Expand Down Expand Up @@ -2320,10 +2320,10 @@ class Strand(Part, JSONSerializable):
"""

def __init__(self,
name: str,
domains: Iterable[Domain] | None = None,
starred_domain_indices: Iterable[int] = (),
group: str = default_strand_group,
name: str | None = None,
label: str | None = None,
vendor_fields: VendorFields | None = None,
) -> None:
Expand All @@ -2348,7 +2348,7 @@ def __init__(self,
"""
self._all_intersecting_domains = None
self.group = group
self._name = name
self.name = name

# XXX: moved this check to Design constructor to allow subdomain graphs to be
# constructed gradually while building up the design
Expand Down Expand Up @@ -2666,7 +2666,7 @@ def name(self) -> str:
if self._name is None:
self._name = self.domain_names_concatenated()
return self._name
Copy link
Member

Choose a reason for hiding this comment

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

Why are there two return statements one after the other here?

# return self.domain_names_concatenated() if self._name is None else self._name
return self.domain_names_concatenated() if self._name is None else self._name

@name.setter
def name(self, new_name: str) -> None:
Expand Down Expand Up @@ -3271,11 +3271,11 @@ def from_json_serializable(json_map: Dict[str, Any]) -> Design:
return Design(strands=strands)

def add_strand(self,
name: str,
domain_names: List[str] | None = None,
domains: List[Domain] | None = None,
starred_domain_indices: Iterable[int] | None = None,
group: str = default_strand_group,
name: str | None = None,
label: str | None = None,
idt: VendorFields | None = None,
) -> Strand:
Expand Down Expand Up @@ -3315,6 +3315,11 @@ def add_strand(self,
:return:
the :any:`Strand` that is created
"""
for strand in self.strands:

if strand.name == name:
raise ValueError(f"A strand with the name '{name}' already exists.")

if (domain_names is not None and not (domains is None and starred_domain_indices is None)) or \
(domain_names is None and not (domains is not None and starred_domain_indices is not None)):
raise ValueError('exactly one of domain_names or '
Expand Down Expand Up @@ -4132,14 +4137,16 @@ def check_names_unique(self) -> None:
self.check_domain_pool_names_unique()

def check_strand_names_unique(self) -> None:
strands_by_name = {}
strands_by_name: Dict[str, Strand] = {}
for strand in self.strands:
name = strand.name
if name in strands_by_name:
raise ValueError(f'found two strands with name {name}:\n'
f' {strand}\n'
f'and\n'
f' {strands_by_name[name]}')
else:
strands_by_name[name] = strand

def check_domain_pool_names_unique(self) -> None:
# self.domain_pools() already computed by compute_derived_fields()
Expand Down
Loading
Loading