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

Correct variable name overlap in int2cart #224

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

joaomcteixeira
Copy link
Member

Corrects a variable name overlap in int2cart in build. There was no bug, but the var names were prone to bugs in the future. Better correct them now.

@joaomcteixeira joaomcteixeira added the bug Something isn't working label Jun 14, 2022
@joaomcteixeira joaomcteixeira self-assigned this Jun 14, 2022
@joaomcteixeira joaomcteixeira requested a review from menoliu June 14, 2022 17:48
@JerryJohnsonLee
Copy link
Contributor

I am not sure this is exactly the best thing to do.. The reason I make it the same name as the original bond length list is that they serve the same purpose: a list of bond lengths for the current residue in order of [N-CA, CA-C, C-N]. If we change this name then in the future if we need to use this list we need to do additional if-statements to decide which list to use depending on the bgeo strategy, right?

@menoliu
Copy link
Collaborator

menoliu commented Jun 15, 2022

I cannot foresee any bugs that may arise from this but I think having a different variable name to store different angles/lengths from different strategies is just better programming form? Also if we're to change bond_lens we should also change bend_angles to follow the same format (though I can see why you didn't change it because _bend_angle and not _bend_angles is used for the rest

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants