Skip to content

Conversation

@josemduarte
Copy link
Contributor

Should fix #715

@josemduarte josemduarte requested a review from lafita December 8, 2017 01:18
josemduarte added a commit to josemduarte/biojava that referenced this pull request Dec 8, 2017
@lafita lafita added the bug Bugs and bugfixes label Dec 8, 2017
Copy link
Member

@lafita lafita left a comment

Choose a reason for hiding this comment

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

It looks great! I only have one minor suggestion!


}
for (Group altG : group.getAltLocs()) {
for (Atom atom : altG.getAtoms()) {
Copy link
Member

Choose a reason for hiding this comment

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

Why not using translate(g, v)? It would be a recursive call to this function, nicer solution.


}
for (Group altG : group.getAltLocs()) {
for (Atom atom : altG.getAtoms()) {
Copy link
Member

Choose a reason for hiding this comment

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

Recursive call to transform(g, m) could also be possible here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, now changed

@josemduarte josemduarte merged commit f18ca28 into biojava:master Dec 12, 2017
@lafita lafita added this to the BioJava 5.0.0 milestone Mar 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Bugs and bugfixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Calc.transform(Chain) does not transform alt groups

2 participants