tickets/PREOPS-4896: make schedview's make_unique_survey_name more robust w.r.t. survey.observations type

[ehneilsen]

The "note" in survey.observations syntax doesn't work when survey.observations is a numpy.recarray, but it can be such in the new auxtel scheduler, and schedview really aught to handle this correctly. Using a try/except block lets schedview be more versatile. This is another instance of "better to ask forgiveness" being better than "look before you leap" in python.

[rhiannonlynne]

Do you mind explaining the logic for the unique survey name?
I see this:

    try:
        survey_name = survey.survey_name
        if len(survey_name) < 1:
            survey_name = str(survey)
    except AttributeError:
        survey_name = str(survey)

    try:
        observation_note = f"{survey.observations['note'][0]}"
    except (AttributeError, ValueError, TypeError):
        observation_note = None

    if (observation_note is not None) and (survey.survey_name != observation_note):
        survey_name = f"{survey.observations['note'][0]}"

    survey_name = f"{survey_index[1]}: {survey_name}"

    # Bokeh tables have problems with < and >
    survey_name = survey_name.replace("<", "").replace(">", "")

Maybe I'm reading this wrong, but doesn't this basically ignore the survey_name which is provided by the survey itself?
The survey.survey_name will almost always be different than the observation.note (because we're now setting the survey_name more properly, and setting it further upstream) and the observation.note should always be set, so this looks like it will always overwrite survey.survey_name.
Is there a problem with using survey.survey_name?

[ehneilsen]

Do you mind explaining the logic for the unique survey name?

This was the behavior requested by the auxtel team, @tribeiro I think. I think it would be a lot clearer if we always used survey_name, but that isn't what they want (or at least, wanted).

I think I did use survey_name originally, until I was asked to make it use note like this.

[rhiannonlynne]

Urg. Uh, I guess they don't want to use "survey_name" either, because they're assigning the target name when it leaves the FBS but they're all part of the same "survey".

I think you might want to make some really clear comment lines here about what it's doing (that it's intentional) and why it's doing that. Otherwise, this just looks like a mistake when it's actually intentional.

We might need to revisit this when we actually properly pass out "survey_name", "target" and "note" ... currently we're combining target and note to be the same, but they should not be. (do you already get "observation.target"? I'm not clear on how far that's propagating at the moment)

[ehneilsen]

(do you already get "observation.target"? I'm not clear on how far that's propagating at the moment)

Experimenting with a scheduler pickle from last night, the "target" field exists, but is an empty string. The actual field name is in "note".

[ehneilsen]

I think you might want to make some really clear comment lines here about what it's doing (that it's intentional) and why it's doing that.

Comment added.

[rhiannonlynne]

Ok, good to know! the "pipes" aren't there to connect 'target' to the thing it should be, I think.
(keep in mind, this should change in the future).