Skip to content

undoes part of #8108 that was incorrect#10784

Merged
nden merged 1 commit into
astropy:masterfrom
chris-simpson:bugfix/moffat2d
Oct 5, 2020
Merged

undoes part of #8108 that was incorrect#10784
nden merged 1 commit into
astropy:masterfrom
chris-simpson:bugfix/moffat2d

Conversation

@chris-simpson

Copy link
Copy Markdown
Contributor

Description

This pull request is to address #8094 which was not a bug, but a bug was introduced by PR #8108

The Moffat2D gamma derivative is now (once again) correctly calculated, as shown in comments at the end of #8094

@astropy-bot astropy-bot Bot added the modeling label Sep 29, 2020
@saimn saimn added the Bug label Sep 30, 2020
@saimn saimn added this to the v4.0.2 milestone Sep 30, 2020
@pllim pllim requested a review from nden September 30, 2020 14:55
@pllim

pllim commented Sep 30, 2020

Copy link
Copy Markdown
Member

Thanks! cc @ycopin

Needs a change log.

Does this need a test as well? Why wasn't this bug caught the last time?

@chris-simpson

Copy link
Copy Markdown
Contributor Author

It's not technically a bug because the code does what it's designed to do, it's just that design is wrong. So a test won't really help (IMHO) because if there had been a test that the change would cause to fail, the claim would have been made that the test was wrong because it was testing the wrong thing. In the original issue, there was a request for a proof of the revised (incorrect) formula, which was never presented so I guess there was a bit of misplaced trust.

I've put the PR# in the changelog (in the right place at the second time of asking) and squashed commits so this ought to be good to go if you're happy with the algebraic proof presented in #8094

@nden

nden commented Oct 5, 2020

Copy link
Copy Markdown
Contributor

Thanks @chris-simpson! I think the confusion came from 1/gamma**2 being included in rr_gg.

@nden nden merged commit a7ee1ac into astropy:master Oct 5, 2020
astrofrog pushed a commit that referenced this pull request Oct 6, 2020
astrofrog pushed a commit that referenced this pull request Oct 6, 2020
eteq pushed a commit to eteq/astropy that referenced this pull request Oct 12, 2020
eteq pushed a commit that referenced this pull request Oct 13, 2020
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.

4 participants