Remove Hydrogen isotopes in deprotonating

Ruibin-Liu commented 10 months ago

Hydrogen isotopes should also be removed. Also, the documentation was old/wrong and now it is updated.

Reference Issues/PRs

NA

What does this implement/fix? Explain your changes

Removing H atoms by element_symbol == "H" doesn't remove isotopes D or T. Simply adding D and T to the list should work.

What testing did you do to verify the changes in this PR?

NA

Pull Request Checklist

[x] Added a note about the modification or contribution to the ./CHANGELOG.md file (if applicable)
[x] Added appropriate unit test functions in the ./graphein/tests/* directories (if applicable)
[x] Modify documentation in the corresponding Jupyter Notebook under ./notebooks/ (if applicable)
[ ] Ran python -m py.test tests/ and make sure that all unit tests pass (for small modifications, it might be sufficient to only run the specific test file, e.g., python -m py.test tests/protein/test_graphs.py)
[x] Checked for style issues by running black . and isort .

Ruibin-Liu commented 10 months ago

What kinds of tests should I add?

sonarcloud[bot] commented 10 months ago

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

a-r-j commented 9 months ago

Thanks @Ruibin-Liu ! I think the tests you've added seem sufficient to me; anything you're unhappy with?

Ruibin-Liu commented 9 months ago

Thanks @Ruibin-Liu ! I think the tests you've added seem sufficient to me; anything you're unhappy with?

I think it's good to go, but I am not sure why some checks failed.

sonarcloud[bot] commented 9 months ago

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

a-r-j commented 8 months ago

Thanks @Ruibin-Liu ! I think the tests you've added seem sufficient to me; anything you're unhappy with?

I think it's good to go, but I am not sure why some checks failed.

Looks like there was some dependency drift. If you sync up to master I expect these tests will pass :)

sonarcloud[bot] commented 8 months ago

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

a-r-j / graphein