sot / mica

Microscope on Chandra aspect
https://sot.github.io/mica
BSD 3-Clause "New" or "Revised" License
3 stars 0 forks source link

add optional dark_cal_ids argument to dark_cal.get_dark_cal_id #276

Closed javierggt closed 2 years ago

javierggt commented 2 years ago

Description

This PR adds an optional argument to dark_cal.get_dark_cal_id. Normally one calls this function like this:

dark_cal.get_dark_cal_id(time, 'before')

and internally it checks all dark-cal IDs to see which one is "closer" according to the given selection criterion. After this change, one will be able to do

dark_cal_ids = dark_cal.get_dark_cal_ids()
dark_cal.get_dark_cal_id(time, 'before', dark_cal_ids=dark_cal_ids)

The reason for this is that I am modifying aca_view to fetch data from the Ska API, and I would like to reduce the number of calls to the API. In the first call above, one gets the dark-cal ID for a given time, and the call might be repeated many times. This is unnecessary because in most cases there will only be one dark-cal ID that applies to all times.

I would rather get the whole list of dark-cal IDs from the API upfront, and then call the get_dark_cal_id method locally for all times.

Interface impacts

Testing

Unit tests

Unit tests were improved to include this new argument:

Independent check of unit tests by [REVIEWER NAME]

Functional tests

I have been using this from aca_view.

jeanconn commented 2 years ago

"fetching a single dark-cal takes a whopping 1 minute. This is crazy."

Yikes. It seems like if it can be locally fetched in super-tiny times that it should even be able to be served over https relatively quickly. It seems like for the aca_view case you might want to find a way to pass just the pixels cutouts that are needed via the Ska API?

javierggt commented 2 years ago

The issue is fetching a single dark-cal image. I can't imagine it has to take that long. I could do as you say and only fetch a section around the image, but that complicates things. In the end I might just say the dark-cal will not be fetched from the API.

jeanconn commented 2 years ago

Yeah, for me the dark cal cutouts don't add much information so for the no-ska-data case just not including them seems better than working the issue.

javierggt commented 2 years ago

Looks fine but needs documentation of some testing.

Done.