BUG: Return of _logpmf #4049
BUG: Return of _logpmf #4049
Conversation
|
This also needs tests. |
ev-br
added
scipy.stats
needs-work
|
Guys, I did the regression test. |
|
@argriffing I corrected that in other commit... I'm very confusing in use git... sorry I'm doing a mess... |
| @@ -238,6 +238,9 @@ def test_pmf(self): | |||
| # regression test for ticket 1779 | |||
| assert_allclose(np.exp(stats.nbinom.logpmf(700, 721, 0.52)), | |||
| stats.nbinom.pmf(700, 721, 0.52)) | |||
| # regression test for ticjet 4029 | |||
There was a problem hiding this comment.
Although I see it was also done above, just referencing the ticket number isn't really a helpful comment. If you think it needs a comment, something more like "logpmf(0,1,1) shouldn't return nan". Or anything that describes the edge case the test is guarding would be better.
There was a problem hiding this comment.
@ewmoore I updated that comment and merge all commits in just one. But I cant send the code to update this pull request... Can you help me?
There was a problem hiding this comment.
I think it's something to do with your patch-1 branch vs. your master branch. This PR knows only about your patch-1 scipy branch and I think your last commit was to only your master scipy branch.
There was a problem hiding this comment.
Do you just need to force push? (Note the -f).
git push -f https://github.com/helderc/scipy.git patch-1
I wrote the rest of this without reading your post carefully enough. I'm going to leave it here, even though it sounds like you've gotten most of it sorted out already yourself. Thanks for sticking with it through all of this. I know it can be discouraging when contributing seems to require endless fighting with the tools rather than working on the code.
I think @argriffing is right. It I can see the spelling change commit under your master branch on github. What has your work flow looked like? Generally speaking the master branch in your repository isn't a useful thing. What I do tends to look like this:
- Create a clone of my scipy repository
git clone https://github.com/ewmoore/scipy.git - Add the main scipy repository as a remote
git remote add upstream https://github.com/scipy/scipy.git - Create a feature branch that tracks the master branch of the main scipy repository.
git branch feature_name upstream/master - Work, create commits, generally small and not necessarily fit for upstream
git add ...
git commit - Rework commits with
git commit --amendto fix up the last commit or
git rebase --interactive upstream/master
to fix up and squash a series of commits. - Push my changes to github
git push https://github.com/ewmoore/scipy.git feature_name
Ad this point, you can create your pull request, etc on github. - If you need add a commit you can just do steps 4 and 6. If you need to change something but want to keep it a single commit, you'll have to rewrite the history, step 5 and then force push to the same branch, that is use
git push -f https://github.com/ewmoore/scipy.git feature_name(note the-fhere vs. in 6. above.) The force push is necessary because you are replacing the contents of the branch on github rather than adding commits to them.
Note that in all of this the master branch in my scipy repository is not used.
It looks to me as though you've somehow made all but the last change to both your patch-1 branch and your master branch. This spelling change appears to have been committed to only the master branch. Perhaps you had the wrong branch checked out when you made the commit?
|
@ewmoore very nice git workflow write-up. Care to add it somewhere more easily discoverable (hacking.rst?) |
|
I can look at doing that. |
This fixes the bug mentioned in #4029