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 general solution for the PDEs to the docstrings #12370

Closed
wants to merge 7,830 commits into from
Closed

Add general solution for the PDEs to the docstrings #12370

wants to merge 7,830 commits into from

Conversation

nicoguaro
Copy link
Contributor

No description provided.


2] `\eta` as the constant in the solution to the differential equation
`\frac{dy}{dx} = -\frac{b}{a}`
2. `\eta` as the constant in the solution to the differentia
Copy link
Contributor

Choose a reason for hiding this comment

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

typo? differentia

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@gxyd, yes, it seems so. Thanks for the catch.

@gxyd
Copy link
Contributor

gxyd commented Oct 6, 2017

@nicoguaro can you please merge master branch in your PR?

@gxyd gxyd added the PR: author's turn The PR has been reviewed and the author needs to submit more changes. label Oct 6, 2017
@nicoguaro
Copy link
Contributor Author

I think I just did it. Let me know if that works

@gxyd
Copy link
Contributor

gxyd commented Oct 6, 2017

It doesn't seem to have been merged.

@nicoguaro
Copy link
Contributor Author

What should I do then?

@gxyd
Copy link
Contributor

gxyd commented Oct 6, 2017

I can do that for you, that wouldn't be a problem. I just thought may want to do that on your own. Let me know if you want me to fix this for you.

@nicoguaro
Copy link
Contributor Author

Well, I am not well versed in the use of Git. I thought that I had everything done when asked for this PR. But it seems that it is not true.

Yes, please proceed.

@gxyd
Copy link
Contributor

gxyd commented Oct 6, 2017

I'll review the PR tomorrow (its a little late already) and get back to you.

@@ -496,7 +496,12 @@ def pde_1st_linear_constant_coeff_homogeneous(eq, func, order, match, solvefun):

where `a`, `b` and `c` are constants.

The general solution is of the form::
The general solution is of the form:

Copy link
Contributor

@gxyd gxyd Oct 8, 2017

Choose a reason for hiding this comment

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

There is trailing whitespace here, remove that please.

where `a(x, y)`, `b(x, y)`, `c(x, y)` and `G(x, y)` are arbitrary functions
in `x` and `y`. This PDE is converted into an ODE by making the following transformation.
where `a(x, y)`, `b(x, y)`, `c(x, y)` and `G(x, y)` are arbitrary
functions in `x` and `y`. This PDE is converted into an ODE by
Copy link
Contributor

Choose a reason for hiding this comment

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

Also remove unnecessary space after 'functions'.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you mean by "unnecessary space after functions"?

Copy link
Contributor

Choose a reason for hiding this comment

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

There is some extra space after the word 'functions' in the beginning of this line, that isn't needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I see. I'll fix it. Are we suppose to correct the source code to comply with PEP8?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if this is included in PEP8 or not. But since the current tests on travis fail because of this. That's why I suggested to change this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I think I made those changes.

@nicoguaro
Copy link
Contributor Author

@gxyd is there something else that need to be done with this?

@sympy-bot
Copy link

Hi, I am the SymPy bot (v137). I'm here to help you write a release notes entry. Please read the guide on how to write release notes.

❌ There was an issue with the release notes. Please do not close this pull request; instead edit the description after reading the guide on how to write release notes.

  • The <!-- BEGIN RELEASE NOTES --> block was not found

Note: This comment will be updated with the latest check if you edit the pull request. You need to reload the page to see it.

Click here to see the pull request description that was parsed.

<!-- Please give this pull request a descriptive title. Pull requests with descriptive titles are more likely to receive reviews. Describe what you changed! A title that only references an issue number is not descriptive. -->

<!-- If this pull request fixes an issue please indicate which issue by typing "Fixes #NNNN" below. -->

@sylee957 sylee957 removed the PR: author's turn The PR has been reviewed and the author needs to submit more changes. label Feb 19, 2019
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