Added view to enable target selection for manual observing runs #840
Conversation
| # a dictionary indexed by site code. Our purpose here is to verify whether each target is ever | ||
| # visible at lower airmass than the limit from any site - if so the target is considered to be visible | ||
| observable_targets = [] | ||
| for object in targets: |
There was a problem hiding this comment.
This should probably be for target in targets:
object is a python built-in
phycodurus
requested review from
phycodurus
and removed request for
phycodurus
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.
| # if configured in the TOM's settings.py. So we set the list of table columns accordingly. | ||
| table_columns = [ | ||
| 'Target', 'RA', 'Dec', 'Site', 'Min airmass' | ||
| ] + settings.SELECTION_EXTRA_FIELDS |
There was a problem hiding this comment.
This will break if there is no settings.SELECTION_EXTRA_FIELDS.
You can replace this with getattr(settings, 'SELECTION_EXTRA_FIELDS', []) to use an empty list if that setting isn't present.
| ] | ||
|
|
||
| # Extract any requested extra parameters for this object, if available | ||
| for param in settings.SELECTION_EXTRA_FIELDS: |
There was a problem hiding this comment.
Same comment as above for line 811.
| {% endif %} | ||
| </table> | ||
| </div> | ||
| {% endblock %} |
| required=False) | ||
| facilities = [(x, x) for x in facility.get_service_classes()] | ||
| observatory = forms.ChoiceField(required=True, choices=facilities) | ||
| date = forms.DateTimeField(required=True, help_text='YYYY-MM-DD') |
There was a problem hiding this comment.
forms.DateField(required=True, initial=datetime.date.today, widget=forms.TextInput(attrs={'type': 'date'})) Is a much cleaner way to do what you want here.
(Be sure to import datetime above)
| observable_targets = [] | ||
| for object in targets: | ||
| start_time = datetime.strptime(request.POST.get('date')+'T00:00:00', '%Y-%m-%dT%H:%M:%S') | ||
| end_time = datetime.strptime(request.POST.get('date')+'T23:59:59', '%Y-%m-%dT%H:%M:%S') |
There was a problem hiding this comment.
These two lines are a bit clunky. I think using datetime.timedelta will be more clear and allow us to more easily work with this code in the future.
i.e.
start_time = datetime.strptime(request.POST.get('date'), '%Y-%m-%d')
end_time = start_time + timedelta(days=1)
Again, dodn't forget to import timedelta up above.
| # table_columns.append(param) | ||
|
|
||
| # Calculate the visibility of all selected targets on the date given | ||
| # Since some observatories include multiple sites, the visibiliy_data returned is always |
| if len(airmass_data) > 0: | ||
| s = SkyCoord(object.ra, object.dec, frame='icrs', unit=(u.deg, u.deg)) | ||
| target_data = [ | ||
| object.name, s.ra.to_string(u.hour), s.dec.to_string(u.deg, alwayssign=True), |
There was a problem hiding this comment.
object.name should be a link back to the target's observe page in case the user wants to actually submit an observation.
| """ | ||
| target_list = forms.ModelChoiceField( | ||
| TargetList.objects.all(), | ||
| required=False) |
There was a problem hiding this comment.
Not requiring a TargetList means that if the user doesn't select anything, then the code will try to do this for EVERY target. This will likely completely crash any TOM with a large DB as it tries to get visibilities for a few million objects.
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.