Skip to content

Conversation

@kpuriIpac
Copy link
Contributor

@kpuriIpac kpuriIpac commented Oct 21, 2025

Ticket: https://jira.ipac.caltech.edu/browse/IRSA-7299
IFE PR: https://github.com/IPAC-SW/irsa-ife/pull/445

  • Updated the upload id from catalogs.owncatalogs to upload.intro

  • This applies to the upload tab as well, and not just the upload dialogs

    • @lrebull if there's a place where the help link should instead go to catalogs.owncatalogs when you test the apps below, please let me know
  • Note: I suppose for Firefly we don't have an upload.intro for the online help, so maybe I should switch back to the default basics.searching for that?

  • Fixes FIREFLY-1826

    • @loitly the bug fix for this is in DataType's formatHeader
      • Bug summary: Essentially, if you have a spectrum table like this (from the ticket)
        • photometry_tbl
      • notice the det_id column. When saving this table as an IPAC table, we convert short to integer, that part works as expected. But when we call fitValueInto inside formatHeader, the width for this column is returned as 6 (because of the det_id string length), and integer is truncated into intege, hence the bug.
      • This fix takes care of that, while still being generic.
        • When the width is longer than than the val length (as is the usual case), for example, for the ra column (not in the screenshot), the width is 11 due to the entries in the column, so we still get a nicely formated "ra " as expected in that case.
        • But in a case like this specific column, it will make sure the data type value is not truncated.

Testing:

  • Firefly, Irsaviewer, DCE, Euclid, Spherex
  • Just test the help link anywhere upload is used takes you to /onlinehelp/#id=upload.intro
    • includes both upload tabs and dialogs
  • For FIREFLY-1826, test according to ticket. Saving the table as an IPAC table should fix the bug in the det_id column. And if you try to re-upload this column to firefly, that should work as well.
    • if Spherex searches take too long, I attached a euclid spectrum table to the ticket. I changed the NDITH column to be datatype short (instead of integer). So if you upload this file to firefly, and try to save the table as an IPAC table, you can test this bug fix that way (and to cross verify, if you upload it on the nightly firefly build: https://fireflydev.ipac.caltech.edu/firefly you'll encounter the bug again)

@kpuriIpac kpuriIpac added this to the 2025.5 milestone Oct 21, 2025
@kpuriIpac kpuriIpac self-assigned this Oct 21, 2025
@kpuriIpac kpuriIpac requested review from loitly and lrebull October 21, 2025 01:18
@lrebull
Copy link
Contributor

lrebull commented Oct 21, 2025

Help anchor: this is definitely fixed for euclid and spherex, which were the ones that were causing the "trouble" with testers. Irsa viewer and dce also work great. so, i need to add the upload.intro anchor to firefly generic help -- that's definitely on me. it should NOT go to basics.

"intege" type: I confirm this is fixed.

thank you!

@kpuriIpac
Copy link
Contributor Author

Help anchor: this is definitely fixed for euclid and spherex, which were the ones that were causing the "trouble" with testers. Irsa viewer and dce also work great. so, i need to add the upload.intro anchor to firefly generic help -- that's definitely on me. it should NOT go to basics.

"intege" type: I confirm this is fixed.

thank you!

Ok so I will leave that as upload.intro for firefly as well, once that anchor is created it'll show up.

@lrebull
Copy link
Contributor

lrebull commented Oct 22, 2025

IN case anyone cares (probably not) ... I went through several minutes of being extra confused . . . i got into the firefly docs, and there sure as heck IS already an upload.intro anchor there.
Screenshot 2025-10-22 at 10 21 26 AM

and there is really an upload.html#intro destination for it too. BUT apparently I never actually put the toc_upload section into the fireflyToc. sigh.

fixed now and checked back into git

Copy link
Contributor

@loitly loitly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix the bad IPAC table export bug as I suggested before merging.

public String formatHeader(String val) {
val = val == null ? "" : val;
return fitValueInto(val, getWidth(), false);
return fitValueInto(val, Math.max(getWidth(), val.length()), false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix doesn’t fully address the issue. The problem occurs where the headers are modified after shrinkToFitData is called. here

I suggest moving that function call to just before the writing step. You may also need to add a second parameter to the function so you can pass in the updated headers for the calculation.

@loitly loitly self-requested a review October 23, 2025 18:09
@kpuriIpac kpuriIpac force-pushed the IRSA-7299-help-fix-upload-dialog branch from 14c254c to 8edd7ee Compare October 25, 2025 00:01
@kpuriIpac
Copy link
Contributor Author

Please fix the bad IPAC table export bug as I suggested before merging.

@loitly I addressed the feedback and updated the test builds. Can you have a quick look before I merge?

Copy link
Contributor

@loitly loitly left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IPAC table changes look good. Thanks for the fix.

@kpuriIpac kpuriIpac force-pushed the IRSA-7299-help-fix-upload-dialog branch from 8edd7ee to 511cb55 Compare October 27, 2025 18:32
@kpuriIpac kpuriIpac merged commit 27ac559 into dev Oct 27, 2025
@kpuriIpac kpuriIpac deleted the IRSA-7299-help-fix-upload-dialog branch October 27, 2025 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants