Skip to content

Refactorings of getAminoAcidMapping#243

Merged
VerisimilitudeX merged 11 commits intoVerisimilitudeX:mainfrom
Speedro:refactoring/get_amino_acid_mapping
Oct 27, 2022
Merged

Refactorings of getAminoAcidMapping#243
VerisimilitudeX merged 11 commits intoVerisimilitudeX:mainfrom
Speedro:refactoring/get_amino_acid_mapping

Conversation

@Speedro
Copy link
Contributor

@Speedro Speedro commented Oct 23, 2022

closes #186

  • updated junit version so we can use parametrized tests
  • created CodonDataUtilsTest class
  • deleted no longer used AminoAcidMapping.java and AminoAcidNames.java.
  • only AminoAcid.java enum should be used

@Speedro Speedro changed the title Closes #186 Refactorings of getAminoAcidMapping, closes #186 Oct 23, 2022
@Speedro Speedro changed the title Refactorings of getAminoAcidMapping, closes #186 Refactorings of getAminoAcidMapping Oct 23, 2022
@VerisimilitudeX VerisimilitudeX self-requested a review October 24, 2022 02:59
Copy link
Owner

@VerisimilitudeX VerisimilitudeX left a comment

Choose a reason for hiding this comment

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

@Speedro looks like there are conflicts again. This PR is first in my backlog, so I'll review it once you clear the merge conflicts. Thanks!

THREONINE("Theornine", "t", "threonine", "thr"),
VALINE("Valine", "v", "valine", "val"),
THRYPTOPHAN("Thryptophan", "w", "tryptophan", "trp");
TRYPTOPHAN("Thryptophan", "w", "tryptophan", "trp"),
Copy link
Owner

Choose a reason for hiding this comment

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

Remove "Thryptophan". I know you didn't put it there and I have no idea how it got there.

Copy link
Owner

Choose a reason for hiding this comment

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

Or actually, remove the h and leave it as an uppercase check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Verisimilitude11 thanks, I resolved the conflicts and left the 'Tryptophan' inside the enum. Please check, if it's ok and let me know if not 🙂 🚀

@Speedro Speedro force-pushed the refactoring/get_amino_acid_mapping branch 2 times, most recently from 63ebced to c9d90ad Compare October 25, 2022 04:57
@VerisimilitudeX VerisimilitudeX self-requested a review October 25, 2022 05:13
*
* @author @Speedro
*/
public class CodonDataUtilsTest {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Verisimilitude11 Ok, I am not sure about the CodeSceneDelta analyzis fail as the strings are valid inputs for the test case. Maybe this analyzis could be softened for test classes or something.

Copy link
Contributor Author

@Speedro Speedro Oct 25, 2022

Choose a reason for hiding this comment

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

Or the codon parts (ATT, ATC etc.) could be turned into String constants. Not sure if that helps though, but maybe could.

Copy link
Owner

Choose a reason for hiding this comment

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

Yeah don't worry about CodeScene, they don't have the most reliable reporting. I'll merge later tomorrow after some testing.

- updated junit version so we can use parametrized tests
- created CodonDataUtilsTest class
- deleted no longer used AminoAcidMapping.java and AminoAcidNames.java.
- only AminoAcid.java enum should be used
@Speedro Speedro force-pushed the refactoring/get_amino_acid_mapping branch from c9d90ad to 0b00539 Compare October 26, 2022 06:12
@VerisimilitudeX VerisimilitudeX merged commit 2ec47ea into VerisimilitudeX:main Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor AminoAcidMapping.getAminoAcidMapping(String)

2 participants