Membrane components, mechanism, and examples #286
Conversation
The addition of all membrane components, transport mechanism, associated unit tests, and an example.
Spelling mistakes
There was a problem hiding this comment.
The notebook runs fine (and all examples are great!) I have some code-specific comments that I have added to the files directly. Some refactoring may be needed there. I also noticed some unit tests are failing: test_membrane_signaling and test_simple_diffusion (variables not defined).
I can take a look at the PR again and possibly also try to refactor some code soon, if needed.
biocrnpyler/components_membrane.py
Outdated
|
|
||
| def update_reactions(self): | ||
| mech_cat = self.get_mechanism('membrane_sensor') | ||
| return mech_cat.update_reactions(self.membrane_sensor_protein, self.response_protein, self.assigned_substrate, self.signal_substrate, self.energy, self.waste, component=self, part_id=self.name) |
There was a problem hiding this comment.
"IntegralMembraneProtein" is another class after this one (also, membrane channel). So, is this description accurate?
biocrnpyler/components_membrane.py
Outdated
| def __init__(self, membrane_protein: Union[Species, str, Component], | ||
| product: Union[Species,str, Component], | ||
| direction= None, size=None, attributes=None, **keywords): | ||
| """Initialize a MembraneChannel object to store membrane channel related information. |
There was a problem hiding this comment.
Should this be "Initialize an IntegralMembraneProtein component"?
There was a problem hiding this comment.
Is "direction" a string that will be used in species attribute?
There was a problem hiding this comment.
Yes, it should be "Initialize an IntegralMembraneProtein component" and has been updated.
There was a problem hiding this comment.
"Direction" is a string, and will be in species attribute. Is this a clarifying question or do I need to address an issue?
biocrnpyler/components_membrane.py
Outdated
|
|
||
| def update_reactions(self): | ||
| mech_cat = self.get_mechanism('membrane_insertion') | ||
| print(self) |
There was a problem hiding this comment.
This print and others have been removed. They were part of the debugging process.
biocrnpyler/components_membrane.py
Outdated
| substrate: Union[Species, str, Component], | ||
| direction=None, internal_compartment='Internal', external_compartment='External', | ||
| ATP=None, cell=None, attributes=None, **keywords): | ||
| """Initialize a Transporter object to store Transport membrane related information. |
There was a problem hiding this comment.
I'm confused: does this initialize a "MembranePump" object?
There was a problem hiding this comment.
Yes, I used a general term as a transport but changed it to a specific object, as Transporter is not a class.
biocrnpyler/components_membrane.py
Outdated
| substrate: Union[Species, str, Component], | ||
| direction=None, internal_compartment='Internal', external_compartment='External', | ||
| cell=None, attributes=None, **keywords): | ||
| """Initialize a Transporter object to store transport membrane related information. |
There was a problem hiding this comment.
What's a Transporter? Is it a class defined somewhere?
There was a problem hiding this comment.
I have updated the language to the specific class.
biocrnpyler/mechanisms_signaling.py
Outdated
|
|
||
| from .mechanism import Mechanism | ||
| from .reaction import Reaction | ||
| from .species import Complex, Species, WeightedSpecies |
There was a problem hiding this comment.
Seems like a few of these are unused, and so can be removed from the imports.
| Mechanism.__init__(self, name, mechanism_type) | ||
|
|
||
| def update_species(self, membrane_sensor_protein, response_protein, | ||
| assigned_substrate, signal_substrate, energy, waste, |
There was a problem hiding this comment.
I wonder if there is a better way to describe the complexes in the code here than naming 1 to 6. Maybe a list or a dict of complexes could be used to make this code a bit more streamlined.
There was a problem hiding this comment.
Would adding a dict i.e. Complexes={"Signal_Sub:Sensor_Protein":complex1, "Signal_Sub:Sensor_Protein:ATP":complex2,...} be sufficient? Then, would this dictionary be used when calling the complexes in the reactions?
biocrnpyler/mechanisms_transport.py
Outdated
| if part_id is None and component is not None: | ||
| part_id = component.name | ||
|
|
||
| if component is None and (kb1 is None or ku1 is None or kb2 is None or ku2 is None or kcat is None or kex is None): |
There was a problem hiding this comment.
It appears to be a remnant from a previous line; ku2 is not a parameter in the membrane protein integration reactions.
|
|
||
| return [integral_membrane_protein, product, complex1, complex2] | ||
|
|
||
| def update_reactions(self, integral_membrane_protein, product, component=None, part_id=None, complex=None, complex2 = None, |
There was a problem hiding this comment.
Similar to my previous comment, I think we should think about representing the complexes and parameters differently. The Parameter class may be helpful here..I can take a look at refactoring this to address this comment.
There was a problem hiding this comment.
If you have time to meet, I would appreciate it, not sure how best to resolve this comment.
Minor changes to PR BuildACell#286 on BioCRNpyler master
These changes do not address the comment about representing the complexes and parameters differently.
This pull request contains membrane components, mechanisms, and a specialized example for each type. Additionally, the component was updated to have a compartment attribute. Unit tests for each new component and mechanism have also been included. This pull request supersedes previous pull requests, which aimed to add membrane components.
This request also is in place of the 1/16/2025 pull request.