Multiple errors create a separate dialog box for each

[astronomerdave]

I imported a CSV file that apparently wasn't correct, but I had to wait for several minutes until all 70 or so dialog boxes were created.

[jenniferwillow]

I need a quick consultation before I work on this. I need the file you used and to discuss what the correct behavior should be. For example, should I stop processing the file after one error? It’s obvious that the error occurred when running the OTM but I don’t know the context well enough. Sincerely, Jennifer

On Dec 6, 2023, at 9:36 AM, David Hale @.**@.>> wrote:

Assigned #45https://github.com/CaltechOpticalObservatories/NGPS/issues/45 to @jenniferwillowhttps://github.com/jenniferwillow.

— Reply to this email directly, view it on GitHubhttps://github.com/CaltechOpticalObservatories/NGPS/issues/45#event-11171618958, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AFCPAKAQZQQBSGVWT4JIADLYICUK5AVCNFSM6AAAAABAJYCZVGVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGE3TCNRRHA4TKOA. You are receiving this because you were assigned.Message ID: @.***>

[astronomerdave]

I did File > Import and selected /home/developer/cshapiro/target_list_OTM.csv

Certainly the root "cause" is the missing RA and DEC but since what I did is not forbidden, having nearly 100 dialog boxes to clear is not good.

[jenniferwillow]

Hi Dave, Thanks for pointing me to the offending file. However, this brings up the question of whether a target lacking RA and Dec is valid. I presume that these are supposed to be calibration observations? I just spent an hour looking at this and the error are coming from the OTM. I have an output handler that looks for the word ERROR in the output line from the OTM. If the handler finds the word ERROR it displays the dialog with the message from the OTM. Unfortunately, the OTM is spitting out twice the number of errors as there are lines in the file (i.e. one error for RA, one error for Dec). I think this needs to be handled by the OTM. Essentially, these error are popping up because they are being passed to the GUI by the OTM. Unless you want me to specifically ignore these errors I’m not sure what the correct behavior is. Sincerely, Jennifer Milburn

On Dec 6, 2023, at 4:01 PM, David Hale @.**@.>> wrote:

I did File > Import and selected /home/developer/cshapiro/target_list_OTM.csv

Certainly the root "cause" is the missing RA and DEC but since what I did is not forbidden, having nearly 100 dialog boxes to clear is not good.

Reply to this email directly, view it on GitHubhttps://github.com/CaltechOpticalObservatories/NGPS/issues/45#issuecomment-1843894136, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AFCPAKAGSY27HIGCGQGVKALYIEBM5AVCNFSM6AAAAABAJYCZVGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBTHA4TIMJTGY. You are receiving this because you were mentioned.Message ID: @.***>

[astronomerdave]

Yes, RA and DEC can be empty fields. Perhaps the question goes also to Chaz with how the OTM will deal with these, so I've added him to this ticket. As far as the Sequencer is concerned, if it encounters empty RA+DEC fields then it carries on with the requested observation without moving the telescope (so there's no slew, no acquire, etc., but everything else).

NB. initially, we had envisioned using special target names as indicators to run a pre-programmed sequence of actions in order to perform specific calibrations. We are no longer going to use exactly that method. Instead, if the target name ends in a specific extension (TBD, but perhaps .script, for example, foo.script) then the Sequencer will run the script foo.script which is stored in a specific directory. When running scripts like that, the Sequencer is being driven by the script, rather than by the database table. I suppose the OTM will have to skip those and pretend they don't exist; it would be tricky (if even possible) for the ETC to determine how long the script would take.

But in any case, the short answer is yes, empty RA+DEC fields should be allowed.

[chazshapiro]

Empty RA and DEC are allowed by OTM only for special target names like CALIBRATE, FLAT, DARK, etc. It is "dangerous" to skip rows with missing data by default since the user may not notice that's what happened, even if the target gets flagged in the GUI (maybe you'd have to scroll to see it). If we want this behavior we'd have to consult the group.

When there's an OTM error, it prints all the info at once, so if there are many windows it may be that the java is choosing to window-ize each line separately. I'm not sure we need the OTM error to go in the window. Maybe just direct the user to the error pane and dump all the errors there.

[chazshapiro]

BTW this is not even remotely a valid input file -- it's an output file

/home/developer/cshapiro/target_list_OTM.csv

It did not even have RA and DEC columns

[chazshapiro]

Also, when not doing a calibration, having the OS use the RA and DEC of the previous target in the case of blanks could cause unintended consequences when moving rows around

[jenniferwillow]

Hi Chaz, The Java code attaches an output handler to the output stream of the OTM. If the current line contains the word ERROR the code spawns a dialog displaying the error and indicating the offending line (it gets this information from the OTM). This was really designed to help people who have a few errors in their input file to identify and correct them. It certainly wasn’t intended to catch two error for each line. This is really a conflict between the OTM output and the way that the GUI handles that output. The current behavior is clear incorrect but what should the correct behavior be? At this point I’m not sure what to do without some discussion. Sincerely, Jennifer

On Dec 8, 2023, at 9:07 AM, Chaz Shapiro @.**@.>> wrote:

Empty RA and DEC are allowed by OTM only for special target names like CALIBRATE, FLAT, DARK, etc. It is "dangerous" to skip rows with missing data by default since the user may not notice that's what happened, even if the target gets flagged in the GUI (maybe you'd have to scroll to see it). If we want this behavior we'd have to consult the group.

When there's an OTM error, it prints all the info at once, so if there are many windows it may be that the java is choosing to window-ize each line separately

— Reply to this email directly, view it on GitHubhttps://github.com/CaltechOpticalObservatories/NGPS/issues/45#issuecomment-1847537755, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AFCPAKD3KAJZQVZ5INV2ENLYINCODAVCNFSM6AAAAABAJYCZVGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNBXGUZTONZVGU. You are receiving this because you were mentioned.Message ID: @.***>

[astronomerdave]

Empty RA and DEC are allowed by OTM only for special target names like CALIBRATE, FLAT, DARK, etc.

We decided not to use pre-defined names like this (CALIBRATE, FLAT, DARK, etc.) but instead use scripts. So, instead of looking for the above target names, look for all targets ending in .script.

Or what about this alternative: If the target name ends in .script then don't even send it to the OTM? Since the OTM isn't going to process it anyway then it seems better to not require it to handle something its not intended to use in the first place.

BTW this is not even remotely a valid input file -- it's an output file

The particular file is not important but the fact that one is permitted to import a .CSV file without coordinate columns and induce ~100 dialog boxes. You can tell me not to use that file, but now that we know there's a way for a future user to significantly disrupt the software then we need to prevent that.

The Java code attaches an output handler to the output stream of the OTM. If the current line contains the word ERROR the code spawns a dialog displaying the error and indicating the offending line (it gets this information from the OTM)....The current behavior is clear incorrect but what should the correct behavior be?

I suspect that you are making an equivalent to a system call and therefore can't stop the OTM from running once it's started. If that is the case then can you wait until all lines are processed by the OTM before the GUI reports any errors, and then report only one error? Or, as I think Chaz already suggested, report all of the errors but only in one window?

I still think it might be better not to send known non-stellar targets to the OTM in the first place, but trapping errors all in one place might also be a good idea for the future fault conditions we haven't caught yet.

[chazshapiro]

RE: the main issue of the 100 popup boxes, I think we're in agreement to just display all of the OTM errors in the text pane, while the popup should just say something like "There were OTM errors -- check the pane"

RE: what a legal target list now looks like with these scripts, let's have a non-github discussion

[chazshapiro]

Oh and the OTM errors all get printed at the same time, followed by the program ending (if errors exist).

[chazshapiro]

Looks fixed - thanks Jennifer