Fixes #1643 : Added modes for StructureData.get_composition #5926
Conversation
There was a problem hiding this comment.
Thanks for your contribution @kartikeysaran . I have left a number of comments in the code. In addition to that, whenever you add new functionality, it is good practice to add unit tests so we know it works as intended. For this function, you can add tests to tests/orm/nodes/data/test_structure.py.
Hi @sphuber Thankyou for reviewing my pr, I have made the suggested changes. |
There was a problem hiding this comment.
Thanks @kartikeysaran . The tests were failing. I have suggested some corrections that I think should fix it, but I haven't tested it locally.
sphuber
force-pushed
the
my_first_contribution
branch
from
22f4749
to
bb8b720
Compare
@sphuber Thankyou very much for the reviewing my pr, looking forward to learning and contributing more to the community :) |
The `mode` argument allows changing the format. It supports the values `full`, `reduced` and `fractional`: * `full`: The default, the counts are left unnnormalized. * `reduced`: Counts are renormalized to the greatest common denominator. * `fractional`: Counts are renormalized such that the sum equals 1. The default is `full` which maintains backwards compatibility.
sphuber
force-pushed
the
my_first_contribution
branch
from
bb8b720
to
65c9679
Compare
This Pull request fixes the issue #1643
Work done : Added an extra parameter type in the get_compostion() function
Type accept 3 values :
0 : returns the default composition
1 : returns the reduced composition
2 : returns the fractional composition