Added view to enable target selection for manual observing runs #840
Conversation
phycodurus
requested review from
phycodurus
and removed request for
phycodurus
jchate6
left a comment
jchate6
left a comment
There was a problem hiding this comment.
I've made some specific comments, that I think are important, but I have some general comments that I think will require a larger discussion.
Specifically, I'd like to talk about what you are actually looking to accomplish here. This returns a selection of targets in a list that have an airmass < 2.0 within a specific 24 hour window with 10 steps starting at YYYY-MM-DDT00:00:00 for a list of telescopes at a specific observatory. That feels like a highly specific use case that should probably be made more generalized. Users might want to change the limiting airmass, or see all of the telescopes at once, or set up their window so it encompasses a full night at CPT rather than splitting it.
Additionally, I feel like the information returned is limiting. This doesn't tell a user WHEN the target will be at it's lowest airmass, or sort them in any way or give a user an idea of how long the target will be up. Also, this is completely incompatible with non-sidereal targets.
Ultimately I'm wondering if this whole idea makes more sense being incorporated into the target list view. An airmass filter could possibly be added, and "observation planning" columns could be put on the table.
Would love to discuss further when you have time.
tom_targets/templates/tom_targets/target_facility_selection.html
Outdated
Show resolved
Hide resolved
|
I've implemented all of the changes requested above. I take your point about the potential for more generalized functionality for this view but I think it makes sense to implement this version first before trying to generalize. |
jchate6
left a comment
jchate6
left a comment
There was a problem hiding this comment.
I've pointed out a few broken or problematic changes. I haven't really gotten back into the actual utility of the feature. It seems like there is some pagination issue, but I can't really tell if that's just a symptom of other problems.
…idereal_visibility function
…ture/facility_target_selection
jchate6
left a comment
jchate6
left a comment
There was a problem hiding this comment.
Sorry to go another round, but I think this is looking much better!
Let me know if you have any questions.
| visibiliy_intervals, airmass_max, | ||
| observation_facility=request.POST.get('observatory') | ||
| ) | ||
| for site, vis_data in visibility_data.items(): |
There was a problem hiding this comment.
This works well for non-LCO facilities, but takes a very long time to load for just 100 targets at LCO and I'm not sure the table is useful.
jchate6
left a comment
jchate6
left a comment
There was a problem hiding this comment.
Looks great.
Let's have a quick chat about pagination and the changes I've made.
…ture/facility_target_selection
github-project-automation
bot
moved this from Needs Review
to Merged (to dev)
in TOM Toolkit
This feature aims to improve the TOM's useability for observers operating more traditional, block-scheduled observing facilities, often manually or through remote operations. The feature provides a form and view to enable them to easily identify targets visible at their facility on the date indicated.