Skip to content

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