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

add GetStringVectProp() to SubstanceGroup class #3251

Merged

Conversation

greglandrum
Copy link
Member

If this isn't present then the DATAFIELDS property of SubstanceGroups cannot be accessed from Python

As an aside: we should come up with a more systematic (and automated) way of adding the GetXXXProp() methods to classes in Python

@greglandrum greglandrum added this to the 2020_03_4 milestone Jun 24, 2020
@greglandrum greglandrum requested a review from ricrogz June 24, 2020 11:32
Copy link
Contributor

@ricrogz ricrogz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

As an aside: we should come up with a more systematic (and automated) way of adding the GetXXXProp() methods to classes in Python.

Yes, that would be interesting.

Copy link
Contributor

@d-b-w d-b-w left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use boost::python inheritance and add these methods to wrapping for RDProps?

python::class_<SubstanceGroup, boost::shared_ptr<SubstanceGroup>, bases<RDProps>>(

@bp-kelley
Copy link
Contributor

bp-kelley commented Jun 24, 2020

RDProps at one point wasn't a separated class so we haven't wrapped it yet. This is my preferred approach.

We do have easy to use templates currently though. the Mol in the names is an artifact. We don't have a template for GetPropNames yet. Make sure you implement GetPropsAsDict, this is a useful function that only lives in python.

         .def("HasProp", MolHasProp<SubstanceGroup>,
             "Queries a substance group to see if a particular property has been "
             "assigned.\n\n"
             "  ARGUMENTS:\n"
             "    - key: the name of the property to check for (a string).\n")
        .def("GetProp", GetProp<SubstanceGroup, std::string>,
             "Returns the value of the property.\n\n"
             "  ARGUMENTS:\n"
             "    - key: the name of the property to return (a string).\n\n"
             "  RETURNS: a string\n\n"
             "  NOTE:\n"
             "    - If the property has not been set, a KeyError exception "
             "will be raised.\n")
        .def("GetDoubleProp", GetProp<SubstanceGroup, double>,
             "Returns the double value of the property if possible.\n\n"
             "  ARGUMENTS:\n"
             "    - key: the name of the property to return (a string).\n\n"
             "  RETURNS: a double\n\n"
             "  NOTE:\n"
             "    - If the property has not been set, a KeyError exception "
             "will be raised.\n")
        .def("GetIntProp", GetProp<SubstanceGroup, int>,
             "Returns the integer value of the property if possible.\n\n"
             "  ARGUMENTS:\n"
             "    - key: the name of the property to return (a string).\n\n"
             "  RETURNS: an integer\n\n"
             "  NOTE:\n"
             "    - If the property has not been set, a KeyError exception "
             "will be raised.\n")
        .def("GetUnsignedProp", GetProp<SubstanceGroup, unsigned int>,
             "Returns the unsigned int value of the property if possible.\n\n"
             "  ARGUMENTS:\n"
             "    - key: the name of the property to return (a string).\n\n"
             "  RETURNS: an unsigned integer\n\n"
             "  NOTE:\n"
             "    - If the property has not been set, a KeyError exception "
             "will be raised.\n")
        .def("GetBoolProp", GetProp<SubstanceGroup, bool>,
             "Returns the Bool value of the property if possible.\n\n"
             "  ARGUMENTS:\n"
             "    - key: the name of the property to return (a string).\n\n"
             "  RETURNS: a bool\n\n"
             "  NOTE:\n"
             "    - If the property has not been set, a KeyError exception "
             "will be raised.\n")
        .def("ClearProp", MolClearProp<SubstanceGroup>,
             "Removes a property from the molecule.\n\n"
             "  ARGUMENTS:\n"
             "    - key: the name of the property to clear (a string).\n")

        .def("ClearComputedProps", MolClearComputedProps<SubstanceGroup>,
             "Removes all computed properties from the molecule.\n\n")

        .def("GetPropsAsDict", GetPropsAsDict<SubstanceGroup>,
             (python::arg("self"), python::arg("includePrivate") = false,
              python::arg("includeComputed") = false),
             "Returns a dictionary populated with the molecules properties.\n"
             " n.b. Some properties are not able to be converted to python "
             "types.\n\n"
             "  ARGUMENTS:\n"
             "    - includePrivate: (optional) toggles inclusion of private "
             "properties in the result set.\n"
             "                      Defaults to False.\n"
             "    - includeComputed: (optional) toggles inclusion of computed "
             "properties in the result set.\n"
             "                      Defaults to False.\n\n"
             "  RETURNS: a dictionary\n")


@greglandrum greglandrum merged commit 98add2c into rdkit:master Jun 25, 2020
@greglandrum greglandrum deleted the fix/cannot_use_sgroup_fielddata branch June 25, 2020 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants