Fix issue #5 and add ConfigManager

[andrasmatyassy]

As this is my first contribution, please let me know if I missed any contribution requirements. Happy to adjust and contribute.

[gui11aume]

@derpbuffalo This is amazing! Thanks for taking the time to help!

[gui11aume]

@derpbuffalo Would you be comfortable writing some test cases for the new functionalities? The changes look good already. We will accept them when we have some tests showing that they behave as expected. Let me know if you need some guidance. Otherwise, we can also take care of the tests but that will take some time.

[andrasmatyassy]

Hey, sure I can write the tests as well, happy to do it next week!

…

On Fri, Sep 20, 2024, 16:04 Guillaume Filion ***@***.***> wrote: @derpbuffalo <https://github.com/derpbuffalo> Would you be comfortable writing some test cases for the new functionalities? The changes look good already. We will accept them when we have some tests showing that they behave as expected. Let me know if you need some guidance. Otherwise, we can also take care of the tests but that will take some time. — Reply to this email directly, view it on GitHub <#9 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BLMJMHL2SRIU2XGNTVJFLEDZXQTPZAVCNFSM6AAAAABOSCKNQGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRTHAYTSNBUGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[Adibvafa]

@derpbuffalo Looks great! A few question/suggestions:

• How can a user specify what type of mapping they preffer (or even no mapping?)
• ConfigManager could be a generic class, and a more specific ProteinConfig could inherit from it and be defined. This way we can have protein or even predict dna processing to inherit from it.

[gui11aume]

@derpbuffalo I agree with @Adibvafa, this is remarkable work. Thanks so much for your contribution! 👍

[andrasmatyassy]

@Adibvafa
I made ConfigManager and abstract class and added ProteinConfig.

Regarding how a user can specify the mapping:
The mapping type is always a dictionary of lists, the user can achieve following behaviors:

• having ambiguous_aminoacid_behavior set to raise_error (default) in the ProteinConfig is like having no mapping
• having ambiguous_aminoacid_behavior set to standardize_deterministic will always select the first item from the corresponding list in the mapping. If the user wishes, they can override the mapping with ambiguous_aminoacid_map_override to achieve a different result
• having ambiguous_aminoacid_behavior set to standardize_random will select a random item from the corresponding list in the mapping. Again, the user can override the list with ambiguous_aminoacid_map_override

Does this answer your question?

[Adibvafa]

Amazing job on the PR! Once the default behaviour is changed, I will merge it.

[andrasmatyassy]

@Adibvafa I changed the default behavior.

Thank you so much for both of you for your guidance!

[Adibvafa]

@derpbuffalo Great job with the PR!
Some tests seem to have failed, could you run it locally (using pytest) and ensure all test pass?

[andrasmatyassy]

@Adibvafa My bad, I have built in a bug before the final commit, I fixed it and run the tests:

[codecov]

Codecov Report

Attention: Patch coverage is 88.00000% with 12 lines in your changes missing coverage. Please review.

Project coverage is 81.21%. Comparing base (14b2450) to head (ac37266).
Report is 41 commits behind head on main.

Files with missing lines	Patch %	Lines
CodonTransformer/CodonUtils.py	83.33%	8 Missing ⚠️
CodonTransformer/CodonData.py	86.66%	2 Missing ⚠️
tests/test_CodonData.py	95.00%	1 Missing ⚠️
tests/test_CodonUtils.py	94.11%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main       #9      +/-   ##
==========================================
+ Coverage   80.45%   81.21%   +0.76%     
==========================================
  Files           8        8              
  Lines         839      937      +98     
==========================================
+ Hits          675      761      +86     
- Misses        164      176      +12     

Flag	Coverage Δ
unittests	81.21% <88.00%> (+0.76%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Adibvafa]

@derpbuffalo Great job! I will merge the PR. Thank you for your contribution 😄