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

DiscreteDDP: minor fix in docstring and pep8 compliance #198

Merged
merged 2 commits into from
Sep 25, 2015
Merged

Conversation

oyamad
Copy link
Member

@oyamad oyamad commented Sep 25, 2015

Sorry to make such a small fix.

  • Looking at the documentation of DiscreteDP, I realized that I had forgotten to add r at the beginning of the docstring that contains math expressions... An r is added now, and some minor edits are made in docstrings.
  • Wrong indentation is fixed.

@jstac
Copy link
Contributor

jstac commented Sep 25, 2015

@oyamad Thanks.

jstac added a commit that referenced this pull request Sep 25, 2015
DiscreteDDP: minor fix in docstring and pep8 compliance
@jstac jstac merged commit 079d68b into master Sep 25, 2015
@jstac jstac deleted the ddp-fix branch September 25, 2015 15:15
@oyamad
Copy link
Member Author

oyamad commented Sep 27, 2015

Thanks @jstac.

But there are a few mistakes left. (Sorry, I should have built the document locally; now I have found how to do here.)

  • The math expressions intended to be displayed are not displayed: ..math:: must to be .. math:: (with a space).

I will have to make another PR...

There is one minor issue: if you look at the source code of ddp.py, every thing on line 28 and below is in light blue. This seems to be because the [ on line 26 is not closed and the strings are made "raw" (surrounded by r""" and """). I feel uncomfortable with this and would like to fix it. There are at least the following two resolutions.

  • Avoid \beta \in [0, 1), and write 0 \leq \beta < 1 instead.
  • Remove r before """, and double all the backslashes.

Anybody have any other idea?

@jstac
Copy link
Contributor

jstac commented Sep 29, 2015

@oyamad The first solution seems like the best one.

oyamad added a commit that referenced this pull request Nov 2, 2015
oyamad added a commit that referenced this pull request Nov 6, 2015
@najuzilu najuzilu mentioned this pull request May 11, 2020
1 task
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.

2 participants