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

Fix a few broken docstrings #6480

Merged
merged 4 commits into from
Jun 23, 2023
Merged

Conversation

ptosco
Copy link
Contributor

@ptosco ptosco commented Jun 22, 2023

  • Fix a couple of docstrings that were confusing boost::python (FromArray, FromStringArray); namely, the comment was being parsed as the parameter name, i.e.:
    FromArray((MHFPEncoder)vec, (list)Creates a MHFP vector from a list of unsigned integers.) → _vectj
    FromStringArray(_(MHFPEncoder)vec, (list)Creates a MHFP vector from a list of arbitrary strings.) → _vectj
  • Python keywords should not be used as parameter names (namely, from)
  • Function documentation should not include the function signature

- Python keywords should not be used as parameter names
@ptosco
Copy link
Contributor Author

ptosco commented Jun 22, 2023

@greglandrum the failure here is due to the numpy issue that you found earlier today.

Copy link
Member

@greglandrum greglandrum left a comment

Choose a reason for hiding this comment

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

I think the MHFP thing has a simpler solution.

Code/GraphMol/Fingerprints/Wrap/MHFPWrapper.cpp Outdated Show resolved Hide resolved
@greglandrum
Copy link
Member

@greglandrum the failure here is due to the numpy issue that you found earlier today.

yep, I was doing the review as you sent this gentle reminder. :-)

Copy link
Member

@greglandrum greglandrum left a comment

Choose a reason for hiding this comment

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

LGTM

@greglandrum greglandrum merged commit 4f23cb7 into rdkit:master Jun 23, 2023
10 checks passed
@greglandrum greglandrum added this to the 2023_03_3 milestone Jun 23, 2023
@ptosco ptosco deleted the fix_python_docstrings branch June 23, 2023 11:51
greglandrum pushed a commit that referenced this pull request Aug 17, 2023
* - Fix a couple of docstrings that were confusing boost::python
- Python keywords should not be used as parameter names

* changes in response to review

* removed two leftover BOOST_PYTHON_FUNCTION_OVERLOADS

---------

Co-authored-by: Tosco, Paolo <paolo.tosco@novartis.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants