Fix incorrect parsing of the ATCA FITS header

[markccchiang]

Description

• What is implemented or fixed? Mention the linked issue(s), if any. Closes #1375.

• How does this PR solve the issue? Give a brief summary. The value of SPECSYS from the ATCA FITS header is already provided. So we don't need to parse it from the suffix of VELO-XXX (the string after '-'), which would also result in the failure rendering of the coordinate system.

• Are there any companion PRs (frontend, protobuf)? No.

• Is there anything else that testers should know (e.g. exactly how to reproduce the issue)? The coordinate system for ATCA FITS images can be successfully rendered on the frontend.

Checklist

• [x] changelog updated ~/ no changelog update needed~
• [x] e2e test passing / corresponding fix added / new e2e test created
• [ ] ICD test passing / corresponding fix added / new ICD test created
• [x] ~protobuf updated to the latest dev commit /~ no protobuf update needed
• [x] ~protobuf version bumped /~ protobuf version not bumped
• [ ] added reviewers and assignee
• [ ] GitHub Project estimate added

[pford]

Should the original ctype value VELO-XXX also be unchanged? The issue requested the original header. I think I wrote this code, not sure why I changed the value but maybe a casacore issue at the time.

Also if the SPECSYS header is read before the CTYPE header, then the specsys string will be replaced with the CTYPE value, which is not what we want.

Starting at line 319 in this branch FileExtInfoLoader.cc:

        } else if (value.startsWith("VELO-") && specsys.empty()) {   <== only if not set?
            specsys = value.after('-');
            value = "VELO";     <=== remove this line?
        }

[markccchiang]

I noticed that if I do not remove the string after VELO (from the original VELO-XXX), the x-label of the spectral profile plot can not be shown normally. Like the following snapshot:

with "CTYPE3 = VELO" the x-label is normally shown:

Not sure if this is the AST problem, or just the frontend code issue(?) @kswang1029 @TienHao do you have comments on it?

[TienHao]

Not sure if this is the AST problem, or just the frontend code issue(?) @kswang1029 @TienHao do you have comments on it?

Yeah, the frontend code needs to be modified for VELO-XXX. It does not give spectral type name for VELO-XXX.

@kswang1029, do VELO-XXX have the same spectral type name and unit just like VELO? or they should have other names?

[kswang1029]

Not sure if this is the AST problem, or just the frontend code issue(?) @kswang1029 @TienHao do you have comments on it?

Yeah, the frontend code needs to be modified for VELO-XXX. It does not give spectral type name for VELO-XXX.

@kswang1029, do VELO-XXX have the same spectral type name and unit just like VELO? or they should have other names?

Yes just like VELO. The VELO-XXX is a (old?) way to encode the spectral reference frame. We still refer CUNIT3 for the unit.

[pford]

I approved the code review, and will leave it up to the others whether to keep the change to "VELO" or show the actual CTYPE header value. It sounds like the suffix is outdated, does not add any information since SPECSYS is defined, and requires a workaround for the spectral profile.

[TienHao]

Since the frontend is working with CTYPE3 = VELO in the current PR(no VELO-XXX is sent through headers), I think there is no need to change in the frontend for the spectral type name now.

[YuHsuan-Hwang]

Does this PR still need frontend changes?

[kswang1029]

Does this PR still need frontend changes?

I will check and report back.

[kswang1029]

Does this PR still need frontend changes?

No, we do not need a corresponding frontend change. 👍

[kswang1029]

@confluence ready to merge after fixing the changelog conflict.

[github-actions[bot]]

Package	Line Rate	Health
src.Cache	72%	➖
src.DataStream	44%	➖
src.FileList	67%	➖
src.Frame	36%	❌
src.HttpServer	42%	➖
src.ImageData	28%	❌
src.ImageFitter	83%	✔
src.ImageGenerators	43%	➖
src.ImageStats	75%	✔
src.Logger	37%	❌
src.Main	52%	➖
src.Region	69%	➖
src.Session	4%	❌
src.Table	52%	➖
src.ThreadingManager	67%	➖
src.Timer	85%	✔
src.Util	40%	➖
Summary	46% (8618 / 18802)	➖