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

PSFConvolvedModel2D and Upgrades to Modeling #100

Merged
merged 14 commits into from
Mar 26, 2022

Conversation

robelgeda
Copy link
Contributor

@robelgeda robelgeda commented Mar 21, 2022

Enhancements:
close #57
close #55
close #99

Bug fixes:
fixes #20
fixes #87
fixes #105
fixes #106

This PR introduces upgrades to and deprecates the PSFModel class by introducing the PSFConvolvedModel2D class (model). This PR also moves the fitting code to a modeling folder according to #99. Deprecation warning has been added to PSFModel but it has not been removed. I will be removed in the next version of the code (likely v0.5, see ticket #102).

For #87, I exposed the Astropy weights parameter in petrofit.modeling.fitting.fit_model so users can pass fitting weights using rms images. I have added this to the docs as well.

  • Docstrings
  • Docs and examples
  • petrofit_notebooks
  • Tests

@robelgeda robelgeda added Enhancement New feature or request WIP Work in Progress Needs Review Backwards Incompatible labels Mar 21, 2022
@robelgeda
Copy link
Contributor Author

robelgeda commented Mar 22, 2022

Reminder to push changes to petrofit_notebooks once this is merged

@robelgeda robelgeda removed the WIP Work in Progress label Mar 22, 2022
@robelgeda robelgeda modified the milestone: v0.4.0 Mar 22, 2022
@robelgeda
Copy link
Contributor Author

robelgeda commented Mar 25, 2022

To save time, I have added #20, #87 and #105 to this PR (all related to fitting)

@robelgeda robelgeda changed the title PSFConvolvedImageModel PSFConvolvedImageModel and Upgrades to Modeling Mar 25, 2022
@robelgeda robelgeda added Documentation Improvements or additions to documentation Infrastructure labels Mar 25, 2022
@robelgeda robelgeda self-assigned this Mar 25, 2022
@crawfordsm
Copy link

I've taken a look and run through the code. I'm able to reproduce the notebooks, and I like leaving the PSFModel in there for the time being but with the deprecation warning. Setting up the infrastructure to follow the astropy directory structrue make sense and hopefully will make it easier to move some things upstream

@robelgeda robelgeda changed the title PSFConvolvedImageModel and Upgrades to Modeling PSFConvolvedModel2D and Upgrades to Modeling Mar 26, 2022
@robelgeda
Copy link
Contributor Author

Thank you @crawfordsm! I will merge once tests pass. Also I did a minor last minute update, I renamed PSFConvolvedImageModel -> PSFConvolvedModel2D because we may in the future make a 1D version of this class. I will open a ticket as a reminder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment