capitalized variables first to fix bug

[karllark]

It seems that the new astropy modeling framework (v4.0+) passes capitalized variables before the lowercase variables. Not the expected behavior and this caused a bug in the N09 model that impacted the SBL18 model (based on the N09 model).

Closes #48.

[codecov-commenter]

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@9c92f8d). Click here to learn what that means. The diff coverage is n/a.

@@            Coverage Diff             @@
##             master       #52   +/-   ##
==========================================
  Coverage          ?   100.00%           
==========================================
  Files             ?         6           
  Lines             ?       294           
  Branches          ?         0           
==========================================
  Hits              ?       294           
  Misses            ?         0           
  Partials          ?         0           

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 9c92f8d...af557ce. Read the comment docs.

[karllark]

Will be merging this pull request as it fixes the issue.

Tagging @nden and @perrygreenfield in hopes of better understanding what caused this bug.

[perrygreenfield]

I need a little more context. What do you mean by passing variables? As function arguments or what?

[perrygreenfield]

Ok, argument, but I don't see the context of the calling sequence that somehow broke between releases. What was calling it?

[karllark]

This call att_model = N09(ampl=ampl, slope=-0.5, Av=1) worked with older astropy version. At some point (likely v4.0), it started to fail. The issue was that even through Av=1 was set, the Av value being passed to the defined evaluated function def evaluate(self, x, x0, gamma, ampl, slope, Av): changed from Av=1 to Av=-0.5. This was the clue as -0.5 is the passed slope. All I did was change the ordering of the variables in the evaluate function to def evaluate(self, x, Av, x0, gamma, ampl, slope): and then it worked again passing Av=1 and slope=-0.5 as expected.

[perrygreenfield]

I think the order is implicit in the order of the parameter definitions within the class. Perhaps something in the creation has changed that order. I'll take a look.

[karllark]

Does not seem to be the case. Av is defined last.

[perrygreenfield]

Right. But I mean some of the metaclass or init magic may have scrambled that unintentionally.

[karllark]

Ahh..ok. Standing by.

[perrygreenfield]

I took a quick look at it and didn't see anything obvious, but I'm pretty sure there is a problem somewhere. I'll create an issue on modeling and reference this in it. Something somewhere must be changing the order (there is tricky stuff going on internally to preserve the order; no obvious reason why it would change), so I'll have to create a simple example with the same parameters and instrument the code to see where it goes bad. It may be a few days before I can get to it though.

[karllark]

That sounds like a great plan. I'm happy to hear that pointing this out to you is useful. I was worried I just did something incorrectly. Will be interesting to hear if the uppercase/lowercase is the issue or if it is something different.