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

fix broadcasting bug in fitters #5788

Merged
merged 2 commits into from Feb 15, 2017
Merged

fix broadcasting bug in fitters #5788

merged 2 commits into from Feb 15, 2017

Conversation

@nden
Copy link
Contributor

nden commented Feb 9, 2017

Fixes the broadcasting bug in #5737.

@@ -617,3 +617,24 @@ def test_with_fitters_and_sigma_clip(self):
_, fitted_model = fit(g2_init, self.x, self.y, self.z)
assert_allclose(fitted_model.parameters[0:5], self.model_params,
atol=1e-1)

def test_fitters_with_weights():

This comment has been minimized.

Copy link
@mirca

mirca Feb 12, 2017

Member

Looks like it needs @pytest.mark.skipif('not HAS_SCIPY')

@nden nden merged commit dc6bd07 into astropy:master Feb 15, 2017
4 checks passed
4 checks passed
ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls First build on master at 84.205%
Details
@nden nden deleted the nden:fix-lm-with-weigths branch Feb 15, 2017
@@ -225,6 +225,9 @@ Bug Fixes

- Fixed a problem with setting ``bounding_box`` on 1D models. [#5718]

- Fixed a broadcasting problem with weighted fitting of 2D models
with ``LevMarLSQFitter``. [#5737]

This comment has been minimized.

Copy link
@bsipocz

bsipocz Feb 22, 2017

Member

@astrofrog - apparently astrobot was sleeping when this was merged, I'm opening an issue and will try to fix it later.

@astrobot

This comment has been minimized.

Copy link

astrobot commented Feb 22, 2017

@nden - thanks for merging this! However, I noticed the following issue with this pull request:

  • Changelog entry not present (or pull request number missing) and neither the Affects-dev nor the no-changelog-entry-needed label are set

Would it be possible to fix this? Thanks!

If you believe the above to be incorrect (which I - @astrobot - very much doubt) you can ping @astrofrog

@astrofrog

This comment has been minimized.

Copy link
Member

astrofrog commented Feb 22, 2017

@bsipocz - I changed the name of the heroku app the other day and forgot to change it in the astropy settings... anyway I redelivered the payload to test if this PR was correctly flagged, and it all works now.

@bsipocz

This comment has been minimized.

Copy link
Member

bsipocz commented Feb 22, 2017

@astrofrog - Thanks. Mystery of the day is solved then :)

@nden

This comment has been minimized.

Copy link
Contributor Author

nden commented Feb 22, 2017

Of course I did this on purpose, to point out the problem :-). But seriously @bsipocz , how did you spot this?

@bsipocz

This comment has been minimized.

Copy link
Member

bsipocz commented Feb 22, 2017

@nden - I was looking at how we stand in terms of 1.3.1, and the pre-releasing scripts (https://github.com/astropy/astropy-tools/tree/master/pr_consistency) made a nice big red warning about it.

bsipocz added a commit that referenced this pull request Feb 22, 2017
# Non-linear model
g2 = models.Gaussian2D(10, 10, 9, 2, 3)
z = g2(Xin, Yin)
gmod = fitter(models.Gaussian2D(15, 7, 8, 1.3, 1.2), Xin, Yin, z + zsig)

This comment has been minimized.

Copy link
@majkelx

majkelx Dec 25, 2017

Contributor

Contrary to name test_fitters_with_weights, there are no weights used, if I have not overlooked anything...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
7 participants
You can’t perform that action at this time.