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

Create single "Passband" class #61

Closed
OliviaLynn opened this issue Aug 5, 2024 · 2 comments · Fixed by #70
Closed

Create single "Passband" class #61

OliviaLynn opened this issue Aug 5, 2024 · 2 comments · Fixed by #70
Assignees

Comments

@OliviaLynn
Copy link
Member

          I would vote for implementing a class for a single band, `Passband`, instead. Collection of passbands could also be useful, but I don see how we are going to use it practically.
  1. For each observation we need a single passband only, not all of them. It sounds reasonable to me to pass passband object per observation. We also could pass band name, but I see some potential issues with that in the future,
  2. we will need a more flexible interface for passbands which will be able to change transmission with airmass for each individual observation (I think it is what Andy C asked us during LINCC UP). That means that passband name ID will not be enough, because transmission will be parameterized by both filter and airmass value.
  3. It would be harder co combine surveys if we have a single entry point for all passbands

Originally posted by @hombit in #52 (comment)

@OliviaLynn OliviaLynn self-assigned this Aug 5, 2024
@OliviaLynn
Copy link
Member Author

While dealing with above interface changes, address the following as well:

A consideration for a future PR. I think you might be able to simplify the user's workflow a bit if you always normalize the table as soon as it is loaded (instead of having the user manually call calculate_normalized_system_response_tables().

and

Another thought about interface for future - I think we would like to have something like

passbands = Passbands()
sourcemodel.bandflux(passbands,times)

instead of calling the method from passbands.

@OliviaLynn OliviaLynn linked a pull request Aug 20, 2024 that will close this issue
@OliviaLynn
Copy link
Member Author

Screenshot 2024-08-23 at 1 27 36 PM

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 a pull request may close this issue.

1 participant