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

Print residue with insertion code #546

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

smoe
Copy link
Contributor

@smoe smoe commented May 29, 2015

This was a bit of a technical exercise while continuing the thoughts expressed in issue #544 on the visibility of the insertion code. The getFullName method is extended respectively and the python_toString routine falls back to it. The patch required to also update and extend the FullnameType enum for Python.

smoe and others added 4 commits May 29, 2015 11:52
Also got doxygen helped which was erratic on a separation of the ID with a colon.
also now falling back to getFullName method
The enum element ADD_VARIANT_EXTENSIONS_AND_ID_AND_INSERTION_CODE was
not seen, so it became the default.

/homeLvm/moeller/alioth/debian-med-git/ball/source/PYTHON/EXTENSIONS/BALL/residue.sip: In function âObject* slot_Residue___str__(PyObject*)â/homeLvm/moeller/alioth/debian-med-git/ball/source/PYTHON/EXTENSIONS/BALL/residue.sip:96:51: error: âD_VARIANT_EXTENSIONS_AND_ID_AND_INSERTION_CODEâas not declared in this scope
     + sipCpp->getFullName(ADD_VARIANT_EXTENSIONS_AND_ID_AND_INSERTION_CODE)
@dstoeckel
Copy link
Contributor

I think this looks pretty good. One suggestion I would like to make though is to use the FullNameType enum as bitflags that are used to build the combined flags:

enum FullNameType {
    NO_VARIANT_EXTENSIONS = 0,
    ADD_VARIANT_EXTENSIONS = 1,
    ADD_RESIDUE_ID = 2,
    // ADD_VARIANT_EXTENSIONS | ADD_RESIDUE_ID
    ADD_VARIANT_EXTENSIONS_AND_ID = 3,
    ADD_INSERTION_CODE = 4,
    // ADD_INSERTION_CODE | ADD_RESIDUE_ID
    ADD_RESIDUE_ID_AND_INSERTION_CODE = 5,
    // ADD_INSERTION_CODE | ADD_RESIDUE_ID | ADD_VARIANT_EXTENSIONS
    ADD_VARIANT_EXTENSIONS_AND_ID_AND_INSERTION_CODE = 6
}

The combined flags can stay for convenience (and API compatibility), the checks for building the full name would be a lot easier to read though...

@anhi What would you say about that proposal?

@smoe
Copy link
Contributor Author

smoe commented Jun 19, 2015

An ABI incompatibility sounds like a close-to-perfect motivation to perform a library jump from 1.4 to 1.5 :) I was just saying that to satisfy my ambitions for Debian. I have not fully grasped what exactly you were commenting, but if manually assigning values, then the value of ADD_INSERTION_CODE | ADD_RESIDUE_ID | ADD_VARIANT_EXTENSIONS I propose to be 7, not 6 - should also help simplifying the implementation quite a bit.

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.

None yet

2 participants