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

angular resolution function does not respect units of theta in input #192

Closed
maxnoe opened this issue Sep 21, 2022 · 3 comments
Closed

angular resolution function does not respect units of theta in input #192

maxnoe opened this issue Sep 21, 2022 · 3 comments
Labels
bug Something isn't working

Comments

@maxnoe
Copy link
Member

maxnoe commented Sep 21, 2022

Due to how astropy tables work, the input is assumed to be degrees.

@maxnoe maxnoe added the bug Something isn't working label Sep 21, 2022
@HealthyPear
Copy link
Member

apart from the line

result["angular_resolution"] = np.nan * u.deg

which would obviously make for an inconsistent QTable, is there any other buggy line?
I would expect the actual operation of calculating the resolution independent on the quantity unit.

A "democratic" fix could be to modify the function call to provide just the events[theta] column with the decorator

@u.quantity_input(theta=u.deg)

@maxnoe
Copy link
Member Author

maxnoe commented Sep 21, 2022

It's a combination of setting

result["angular_resolution"] = np.nan * u.deg

and then assigning the result of angular_distance which is in rad:

    result["angular_resolution"][index] = by_bin["theta"].groups.aggregate(
        lambda x: np.quantile(x, ONE_SIGMA_QUANTILE)
    )

to something that is not a QTable. Table just does not check / convert units.

@HealthyPear
Copy link
Member

Ah! We are using Table inside the function, not QTable...

So we could just use QTable throughout the whole function and set the special case as

result["angular_resolution"] = np.nan * table["theta"].unit

and no need for any quantity_input decorators

@maxnoe maxnoe closed this as completed in db7ed63 Sep 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants