Closed ryannduma closed 3 months ago
[!WARNING]
Rate limit exceeded
@ryannduma has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 18 minutes and 36 seconds before requesting another review.
How to resolve this issue?
After the wait time has elapsed, a review can be triggered using the `@coderabbitai review` command as a PR comment. Alternatively, push new commits to this PR. We recommend that you space out your commits to avoid hitting the rate limit.How do rate limits work?
CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our [FAQ](https://coderabbit.ai/docs/faq) for further information.Commits
Files that changed from the base of the PR and between f8b7a8aec513830870e987f19df1e93998c8e153 and 55ba0db43bdfd38e5c8388a6921e874791ca9b3d.
The changes introduce two new scripts, VECexample.py
and examples/vec_example.py
, for calculating the Valence Electron Count (VEC) of different chemical compounds. This functionality is facilitated by the new valence_electron_count
function in the smact/properties.py
module, which includes parsing capabilities and error handling. Test coverage is enhanced with a new test function in the smact/tests/test_core.py
file, ensuring proper validation of the new functionality.
Files | Change Summary |
---|---|
VECexample.py, examples/vec_example.py | Introduced scripts for VEC calculation for "Ba5In4Bi5" and "Fe2O3" using the valence_electron_count function from smact.properties . |
smact/properties.py | Added valence_electron_count function for computing VEC with parsing and error handling. |
smact/tests/test_core.py | Added test_valence_electron_count to validate the functionality of the valence_electron_count function, including expected outputs and error handling. |
.pre-commit-config.yaml | Updated pre-commit hooks, modifying nbQA version and Python compatibility argument. |
examples/Crystal_Space/1_reduction.ipynb | Changed import of Iterable from typing to collections.abc for better compatibility. |
In a garden of formulas, oh what a sight,
A rabbit hops joyfully, under moonlight.
Valence counts dancing, a new script in play,
For compounds and elements, come join the fray!
With data from CSV, the VEC's now bright,
A leap for the science, oh what pure delight! šāØ
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
Thank you for submitting this PR @ryannduma . A few things before I can accept this PR.
pip install pre-commit
) and run pre-commit run --all-files
: This automatically lints and formats any updated code to make it compliant with some coding conventions/styles. This will make the PR pass the QA stage of the CIsmact/tests/test_core.py
Element('Fe').num_valence_modified
, which is available on this develop branch.Hey Anthony,
I have resolved all conflicts and the code now passes the qa stage of the CI except its failing all the other tests of the CI, how can I resolve that?
All modified and coverable lines are covered by tests :white_check_mark:
Project coverage is 75.67%. Comparing base (
f7a72b7
) to head (55ba0db
). Report is 206 commits behind head on develop.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
šØ Try these New Features:
Merged!
Hey @AntObi , the code passed the tests after a bunch of minor tweaks here and there to the types of errors raised, I noticed the Lr0 element check passed as a ValueError when passed to the function instead of a TypeError as presumed so I edited that part of the test to match, and it passed, though I dont know if thats exactly right, same to the Xx which isnt a valid element but I guess when parsed over is accepted because of the following codeblock, which ofc down the line, is found to not have a Valence number and then returns a ValueError instead of a NameError - please let me know what your thoughts are on this and whether additional tweaks need to be made.
Thank you and have a good evening
def parse_formula(formula): pattern = re.compile(r'([A-Z][a-z]*)(\d*)') elements = pattern.findall(formula) element_stoich = defaultdict(int) for (element, count) in elements: count = int(count) if count else 1 element_stoich[element] += count return element_stoich
VEC function Pull Request
Description
Hey, I hope this PR finds you well, I have now completed the integration of the VEC function succintly into the smact.properties.py module and attached a relevant example of it's use. I would also like to indicate that the function takes two arguments - the compound in question and a valence file - allowing for the user to select any file say magpie, or any existing valence file that they would like to use, in this case the example uses the modified file you shared.
I would like to highlight that this is a New feature (non-breaking change which adds functionality)
How Has This Been Tested?
Test Configuration: VECexample.py
Reviewers
@AntObi - Admin and Maintainer
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Tests