Skip to content

fix failure in fw location for Astier one-param fit - #34

Open
weatherhead99 wants to merge 1 commit into
mainfrom
PTC_fitting_fwloc_fix
Open

weatherhead99 wants to merge 1 commit into
mainfrom
PTC_fitting_fwloc_fix

Conversation

@weatherhead99

Copy link
Copy Markdown
Contributor

fix failure in fw location for Astier one-param fit, and add support for unc.UFloat columns with no units

@Yashvi-Sharma Yashvi-Sharma left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

nomvals = list(nomvals)
errvals = list(errvals)
nomdtp = _choose_minimal_numpy_dtype(nomvals)
errdtp = _choose_minimal_numpy_dtype(errvals)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this default to float16? Should it be float64

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it checks all values and chooses the minimal dtype that accurately represents them all

return outcols


def _unc_column_to_fits_column(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a helper would be useful here since it uses a lot of the same code in the _quantity_column_to_fits_column function

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, could be nice. This code will be disappearing before Christmas though since the taskresult redesign will replace a bunch of it. But i can put a helper in for now if you like.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants