Skip to content

Firefly-1733: Improve file analysis handling, cleaned up many outstanding loose ends#1759

Merged
robyww merged 1 commit intodevfrom
FIREFLY-1733-analysis
May 9, 2025
Merged

Firefly-1733: Improve file analysis handling, cleaned up many outstanding loose ends#1759
robyww merged 1 commit intodevfrom
FIREFLY-1733-analysis

Conversation

@robyww
Copy link
Contributor

@robyww robyww commented May 7, 2025

Firefly-1733: Improve file analysis handling, cleaned up many outstanding loose ends

  • Spectrum analysis now happens during file analysis - Mostly used for MultiProductViewer - FileUploadViewPanel is slightly smarter
  • Removed unused "strategy" from FITSTableReader
  • better handle multiple service descriptor
  • user can control if a service descriptor defaults to activate or enter paramters
  • better error messages when data is not found
  • Improved analysis of when to automatically call a service descriptor
  • Better handling of catalog files with more than one service descriptor
  • Clean up dataTypeHint, removed unnecessary parameters
  • TAP
    • Fixed tap schema and table navigation
    • for reporting issues: A TAP UWS call will now log a synchronous version url
  • Also fixes: Firefly-1732
  • Also fixes Firefly-1728: update MAST TAP url
  • Also improves wording layout in CoverageViewer

Testing

@robyww robyww added this to the 2025.3 milestone May 7, 2025
@robyww robyww self-assigned this May 7, 2025
@robyww robyww added bug UI Client side UI changes not related to any of the visualizers multi-ticket This PR implements multiple Jira tickets Refactor Refactoring or code cleanup TAP labels May 7, 2025
@robyww robyww requested a review from kpuriIpac May 7, 2025 17:53
@robyww robyww force-pushed the FIREFLY-1733-analysis branch from 39e698d to 13bfde5 Compare May 8, 2025 15:07
Copy link
Contributor

@kpuriIpac kpuriIpac left a comment

Choose a reason for hiding this comment

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

Looking mostly good! You were right, lots of changes so I was not able to go through all of the code but just skim over it. Tested all apps, file uploads seem to work well, didn't really find anything broken in existing behavior.

I found one bug, though. Tried a CADC search in firefly, in the Data Product Viewer, click on More and then select cutout. If you try to edit the BAND field value, you get this error (even if you keep it the same as the default, which is 0).

The exact search I did: CADC -> IVOA -> m81, Row Limit:50, try the first row (obs_collection HST). I think you basically need a cutout to test this.

Copy link
Contributor

@kpuriIpac kpuriIpac left a comment

Choose a reason for hiding this comment

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

Let me know if you want me to specifically test any other behavior in any of the apps.

const doResetButton= displayType!==DPtypes.ANALYZE && !isWorkingState && Boolean(searchParams || serDef?.serDefParams?.some( (sdp) => !sdp.ref));

const getInput= displayType===DPtypes.ANALYZE && allowsInput && !searchParams;
// if (isDefined(serDescActive)) getInput= !serDescActive;
Copy link
Contributor

Choose a reason for hiding this comment

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

might be cleaned up and removed

@robyww robyww force-pushed the FIREFLY-1733-analysis branch 2 times, most recently from 95b6c36 to b5e1ee1 Compare May 9, 2025 15:31
@kpuriIpac kpuriIpac self-requested a review May 9, 2025 16:19
…d loose ends

 - Spectrum analysis now happens during file analysis
     - Mostly used for MultiProductViewer
     - FileUploadViewPanel is slightly smarter
 - Removed unused "strategy" from FITSTableReader
 - better handle mulitple service descriptor
 - user can control if a service descriptor defaults to activate or enter paramters
 - better error messages when data is not found
 - Improved analysis of when to automatically call a service descriptor
 - Better handling of catalog files with more than one service descriptor
 - Clean up dataTypeHint, removed unnecessary parameters
 - TAP
    - Fixed tap schema and table navigation
    - for reporting issues: A TAP UWS call will now log a sychronous version url
 - Also fixes: Firefly-1732
 - Also fiuxes Firefly-1728: update MAST TAP url
 - Also improves wording layout in CoverageViewer
 - includes response to feedback
@robyww robyww force-pushed the FIREFLY-1733-analysis branch from b5e1ee1 to 5ceb780 Compare May 9, 2025 16:43
@robyww robyww merged commit 029440f into dev May 9, 2025
@robyww robyww deleted the FIREFLY-1733-analysis branch May 9, 2025 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug multi-ticket This PR implements multiple Jira tickets Refactor Refactoring or code cleanup TAP UI Client side UI changes not related to any of the visualizers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants