add holo command to generate a holography view

[mattieudv]

Adding a feature to generate a view specifically useful for holography observations.

Do you have a way of testing this in the lab before rolling it out to site?

[bmerry]

Still a few questions open from my first review.

[bmerry]

Eventually we'll get CAM to start using Python 3 and then obs_params will use strings. Instead of changing all this code to use bytes, you can just change how you get obs_params to obs_params = to_str(telstate.get(obs_params_keys, {})), which will convert all the keys and values to strings.

[mattieudv]

yes, they should all be changed in a future PR, to to_str

[bmerry]

Since this is the first time you're using to_str, you cshould start using it right in this PR i.e. use to_str when you get obs_params out of telstate instead of on each access to it.

[bmerry]

To be clear, I'm not suggesting you should try to roll out to_str to the rest of the code - just update the code that's new in this PR.

[mattieudv]

Still a few questions open from my first review.

I do not see these questions - most code blocks are outdated.

I added 'print scan_ants_always to console', would ideally like to merge this branch to master now for today's deployment.

[mattieudv]

Anything outstanding about this, can we merge with master?

…

On Wed, Apr 21, 2021 at 10:23 AM Bruce Merry ***@***.***> wrote: ***@***.**** requested changes on this pull request. ------------------------------ In scripts/time_plot.py <#84 (comment)>: > + obs_params=telstate.get(obs_params_key, {}) + scan_ants_always=[] + scan_ants=[] + track_ants=[] + if b'scan_ants' in obs_params: + scan_ants=to_str(obs_params[b'scan_ants']).split(',') + if b'track_ants' in obs_params: + track_ants=to_str(obs_params[b'track_ants']).split(',') + if b'scan_ants_always' in obs_params: + scan_ants_always=to_str(obs_params[b'scan_ants_always']).split(',') ⬇️ Suggested change - obs_params=telstate.get(obs_params_key, {}) - scan_ants_always=[] - scan_ants=[] - track_ants=[] - if b'scan_ants' in obs_params: - scan_ants=to_str(obs_params[b'scan_ants']).split(',') - if b'track_ants' in obs_params: - track_ants=to_str(obs_params[b'track_ants']).split(',') - if b'scan_ants_always' in obs_params: - scan_ants_always=to_str(obs_params[b'scan_ants_always']).split(',') + obs_params=to_str(telstate.get(obs_params_key, {})) + scan_ants_always=[] + scan_ants=[] + track_ants=[] + if 'scan_ants' in obs_params: + scan_ants=obs_params['scan_ants'].split(',') + if 'track_ants' in obs_params: + track_ants=obs_params['track_ants'].split(',') + if 'scan_ants_always' in obs_params: + scan_ants_always=obs_params['scan_ants_always'].split(',') ------------------------------ In scripts/time_plot.py <#84 (comment)>: > + for ant in track_ants: + if ant in scan_ants_always: + scan_ants.append(ant) + else: + newtrackants.append(ant) + track_ants=newtrackants + scan_ants=sorted(scan_ants) + if (len(args)==1 or args[1]==''): + if len(track_ants): + refantnumber=int(track_ants[0][1:]) + else: + refantnumber=antnumbers[0] + else:#use supplied refant + refantnumberlist=parse_antennarange(','.join(args[1:])) + if (len(refantnumberlist)==1): + refantnumber=refantnumberlist[0] This is a question that you haven't answered. ------------------------------ In katsdpdisp/html/help.txt <#84 (comment)>: > @@ -232,8 +232,9 @@ A spectrum plot is also produced. You may additionally want to issue a command such as 'flags=..500,520..' For example: -'holo' regenerates a view using the first tracking antenna as reference of cross correlation product +'holo' regenerates a view using the first tracking antenna as reference of cross correlation product (h pol) 'holo 4' regenerates a view using m004 as reference of cross correlation product Help should say so then. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#84 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKRCLEH5MURJH7ZZFUZKLTTJ2DOLANCNFSM4ZLHZQTA> .

[mattieudv]

github appears to suck, there is only a resolve option, and no reply option to your comment. In the case you mention the 'else' part will be called of the if statement (i.e. it simply chooses the first antenna in the array): send_websock_cmd('logconsole("Invalid reference antenna specified, using default instead",true,true,true)',handlerkey) refantnumber=antnumbers[0]

…

On Tue, May 4, 2021 at 4:22 PM Bruce Merry ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In scripts/time_plot.py <#84 (comment)>: > + for ant in track_ants: + if ant in scan_ants_always: + scan_ants.append(ant) + else: + newtrackants.append(ant) + track_ants=newtrackants + scan_ants=sorted(scan_ants) + if (len(args)==1 or args[1]==''): + if len(track_ants): + refantnumber=int(track_ants[0][1:]) + else: + refantnumber=antnumbers[0] + else:#use supplied refant + refantnumberlist=parse_antennarange(','.join(args[1:])) + if (len(refantnumberlist)==1): + refantnumber=refantnumberlist[0] This is still outstanding. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#84 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKRCLAVIYMGSVFZWU237QLTL77ITANCNFSM4ZLHZQTA> .

[bmerry]

Hooray, reduced code and it's more robust :-)

[mattieudv]

Merging now