Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make pybel compatible with #1975 #2005

Merged
merged 3 commits into from
Jul 30, 2019
Merged

Make pybel compatible with #1975 #2005

merged 3 commits into from
Jul 30, 2019

Conversation

yishutu
Copy link
Contributor

@yishutu yishutu commented Jul 8, 2019

I have made some change of pybel.py to compatible with function name changes in #1975. Some property names were changed keeping the original name as the alias.
heavyvalence -> heavydegree
heterovalence -> heterodegree

@baoilleach
Copy link
Member

Hi. Good fix, but I would suggest some changes.

I don't think we should keep the legacy names. The whole problem is that valence and degree were mixed up. If we keep the legacy names, we perpetuate the confusion.

Also, "def valence" should be renamed to "def degree", and the Atom docstring should be updated.

old properties will raise the error
@yishutu
Copy link
Contributor Author

yishutu commented Jul 10, 2019

Thanks for suggestion. So there are the changes I have made:

  • property name changes
    • heavyvalence -> heavydegree
    • heterovalence -> heterodegree
    • valence -> degree
  • old property names will raise AttributeError to guide users to use the new property names.
  • doc strings are updated

@ghutchis
Copy link
Member

@baoilleach - ready to go?

@baoilleach baoilleach merged commit 7cfe448 into openbabel:master Jul 30, 2019
@yishutu yishutu deleted the pybel branch August 6, 2019 02:07
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.

3 participants