adding phase dependence to prop. effects

[jpierel14]

@benjaminrose PR to add phase dependence to propagation effects. I don't think this should cause any issues (I've tested phase-dependent, wavelength-dependent, and phase+wavelength-dependent effects), but happy to discuss.

[benjaminrose]

It looks like there are some errors in the testing. I know some are on SNCosmo's side but @jpierel14 does the p39 tests pass locally?

[benjaminrose]

Justin and I talked a bit offline. We noted that making phase be the first parameter (though logical if we were just starting) breaks backwards compatibility (see failing tests). Likely it is best to make phase an optional argument rather than a required positional argument.

[benjaminrose]

@jpierel14 do you know why propagate is defined so many times?

To get this merged, I think we should

• add a test
• shift phase to an optional parameter to keep backwards compatibility (def propagate(self, wave, flux, phase=None):)
• also see if there is a need to refactor how propagate is defined. We might fix that here, or it might be its own issue.

[benjaminrose]

The code style error is an easy fix, and I will do that. Let me also review the changes to look for maintainability problems. Other than that, it looks like we are on the right path to adding this new feature.

[jpierel14]

@benjaminrose Okay new try

[jpierel14]

Look at all those check marks

[benjaminrose]

I have a few documentation additions. I was going to do them myself, but I con't have push access to your repo. I'll just push them to main after I merge this in.

[jpierel14]

Sounds good, thanks a lot!