add chain_id parameter, fix atom name issue, specify torch-fourier-sl… - #9
add chain_id parameter, fix atom name issue, specify torch-fourier-sl…#9pcbve1 wants to merge 2 commits into
Conversation
| unique_chain_ids = self.chain_ids | ||
|
|
||
| for chain_id in unique_chain_ids: | ||
| print(f"Removing chain {chain_id}") |
There was a problem hiding this comment.
Should not have print statements within code, especially looped areas of code (design choice)
There was a problem hiding this comment.
Removed both print statements. Will open an issue and discuss how best to track deleted residues and chains
| for idx in window_iter: | ||
| chain_ids = chains[idx] | ||
| residue_ids = residues[idx] | ||
| print(f"Removing residues {residue_ids[0]} to {residue_ids[-1]}") |
There was a problem hiding this comment.
Same here, no print statements in production code
| "ttsim3d", | ||
| "teamtomo-basemodel", | ||
| "Leopard-EM>=v1.0", | ||
| "torch-fourier-slice>=v0.4.0" |
There was a problem hiding this comment.
Should bump this to ...>=v0.5.2 for all TeamTomo packages as newest release with some minor bug fixes and API updates.
There was a problem hiding this comment.
updated this line, looks to me like torch-fourier-slice and ttsim3d are the only TeamTomo packages explicitly imported by MOSAICs; the rest are dependencies of Leopard-EM. I'll keep the rest in their current versions, only because if the version of torch-fourier-slice is not defined, MOSAICS crashes. I have not encountered this issue for the other packages.
| chain_ids: list[str] = ["all"] | ||
| randomize_chain_order: bool = False | ||
|
|
||
| _chain_order: list[str] | ||
|
|
||
| def __init__(self, **data: Any): | ||
| super().__init__(**data) | ||
|
|
||
| # The unique method should retain default order | ||
| self._chain_order = self.structure_df["chain"].unique() | ||
| # The unique method should retain default order, or an order specified by the user. | ||
| if self.chain_ids == ["all"]: | ||
| self._chain_order = self.structure_df["chain"].unique() | ||
| else: | ||
| self._chain_order = self.chain_ids |
There was a problem hiding this comment.
Nicer approach than having a helper function to set an internal parameter for chain ordering. If chain_ids exist, then we should be able to remove _chain_order and use the new list for iteration.
Two things to note:
- Would prefer a default value of
Nonewhich then draws the unique chain names from the dataframes upon initialization rather than string"all". - Should add assertion / ValueError if any of the strings (chain IDs) are not contained in the DataFrame upon initialization.
2 participants
…ice version