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

Improve documentation of callback functions #274

Merged
merged 5 commits into from
Oct 22, 2024

Conversation

chrhansk
Copy link
Contributor

This should fix #153 and hopefully generally improve readability of the documentation of the callbacks.

@moorepants
Copy link
Collaborator

Thanks for this. I built it but it doesn't seem to work with the numpydoc format:

image

@moorepants
Copy link
Collaborator

I take that back, it does work, but the formatting is a bit unusual with the H1 or H2 in the sub bullet lists.

@moorepants
Copy link
Collaborator

Here is SciPy's solve_ivp docstring that also requires a callback function:

image

That seems sufficient to me and still fits in with the numpydoc format.

May I suggest that at least the "Parameters" and "Returns" headers in the sublist are turned into just bold **Parameters** or not have any formatting applied?

@chrhansk
Copy link
Contributor Author

That is a good suggestion, I was not entirely happy with the formatting either. I made the changes you suggested, please have a look.

This may be some issue with my setup, but it seems to me that part of the list of attributes is cut off and not visible in the documentation for some reason (i.e., the hessianstructure and intermediate sections). Can you confirm this?

@moorepants
Copy link
Collaborator

Thanks. I confirm that part of the docs aren't being rendered:

image

That's odd.

If this callback is not provided, the Hessian is calculated numerically.


**Parameters**
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it somehow because of the multiple blank lines here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

That fixed it for me. Remove one blank line above **Parameters**.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I removed the line and pushed a commit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, appreciate it.

@moorepants moorepants merged commit bc8a034 into mechmotum:master Oct 22, 2024
34 checks passed
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.

Documentation should mention row-major or column-major flattening for jacobian.
2 participants