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

[Enhancement]: Change data.mask convention in Stages 3+ #519

Closed
1 task done
taylorbell57 opened this issue Apr 14, 2023 · 4 comments · Fixed by #697
Closed
1 task done

[Enhancement]: Change data.mask convention in Stages 3+ #519

taylorbell57 opened this issue Apr 14, 2023 · 4 comments · Fixed by #697
Assignees
Labels
enhancement New feature or request

Comments

@taylorbell57
Copy link
Collaborator

taylorbell57 commented Apr 14, 2023

Instrument

Other (any stage)

What is your suggestion?

At present, the Stage 3 code sets data.mask to True where there is good data and data.mask to False where the data is bad. This is the opposite convention of numpy masked arrays and is confusing to understand if you send someone your FluxData.h5 file since most people, including myself, are likely to assume that a point should be masked if data.mask is True. The current convention was moderately convenient at one point since doing flux*mask zeroes out pixels that shouldn't be used, but I'm pretty sure we're using numpy masked arrays throughout now.

Code of Conduct

  • I agree to follow this project's Code of Conduct
@taylorbell57 taylorbell57 added the enhancement New feature or request label Apr 14, 2023
@taylorbell57 taylorbell57 self-assigned this Apr 14, 2023
@kevin218
Copy link
Owner

We will need to thoroughly test any changes that address this enhancement. There are many places in the code were we could introduce a bug by not accounting for this switch.

@kevin218 kevin218 moved this to To do in Road to v1.1 Jul 26, 2024
@taylorbell57
Copy link
Collaborator Author

@kevin218, since this will break backwards compatibility with old save files, I figure this is a now-or-never task (aka not something we can leave for v1.1). I don't think it critical and we could just abandon it, but it is confusing and has tripped me up on occasion. I'm not 100% sure, but I think the masking convention is also the opposite for Stages 4+ compared to Stage 3.

@kevin218
Copy link
Owner

@taylorbell57 I agree that this is a now or never scenario. I support changing data.mask for v1.0.

@taylorbell57
Copy link
Collaborator Author

Alright, I'll aim to have that done before the end of the week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants