Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yuhsuan/stores null check #2373

Open
wants to merge 38 commits into
base: dev
Choose a base branch
from
Open

Conversation

YuHsuan-Hwang
Copy link
Collaborator

Description

This PR is for #2343 migrating part of the files in /stores to strictNullChecks enabled. The rest of the files in /stores will be migrated in a separate PR.

Some additional changes:

  • CatalogPlotWidgetStore fitting, minMaxX, and statistic: set the entire variable to null instead of setting all members to undefined.
  • Adjusted PvGeneratorWidgetStore setSpectralCoordinate and setSpectralSystem.
  • Fixed the returned values in SpectralLineQueryWidgetStore.filters. Return an array of filter strings instead of an array of headers.
  • Fixed incorrect telemetry data in SpectralProfileWidgetStore.requestMoment. Send the info of the selected region (either active or a specific id) instead of the active region.

Checklist

For linked issues (if there are):

  • assignee and label added
  • GitHub Project estimate added

For the pull request:

  • reviewers and assignee added
  • GitHub Project estimate added
  • changelog updated / no changelog update needed
  • unit test added (for functions with no dependenies)
  • API documentation added (for public variables and methods in stores)

For dependencies:

  • e2e test passing / corresponding fix added / new e2e test created
  • protobuf version bumped / no protobuf version bumped needed
  • protobuf updated to the latest dev commit / no protobuf update needed
  • corresponding ICD test fix added (BackendService changed) / no ICD test fix needed (BackendService unchanged)
  • user manual prepared (for large new features)

@YuHsuan-Hwang YuHsuan-Hwang added the awaiting code review For pull requests that require code review label May 2, 2024
Copy link
Collaborator

@kswang1029 kswang1029 left a comment

Choose a reason for hiding this comment

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

As picked up by the e2e tests, when we select a casa image or a miriad image in the file list, the file info cannot be displayed. fits and hdf5 are fine.
Screenshot 2024-05-06 at 13 18 12

@kswang1029 kswang1029 added awaiting code changes For pull requests that require code changes and removed awaiting code review For pull requests that require code review awaiting testing For pull requests that require testing labels May 6, 2024
@YuHsuan-Hwang YuHsuan-Hwang marked this pull request as draft May 8, 2024 03:14
@YuHsuan-Hwang
Copy link
Collaborator Author

Fixed failed e2e tests.

@YuHsuan-Hwang YuHsuan-Hwang marked this pull request as ready for review May 14, 2024 09:01
@YuHsuan-Hwang YuHsuan-Hwang added awaiting code review For pull requests that require code review awaiting testing For pull requests that require testing and removed awaiting code changes For pull requests that require code changes labels May 14, 2024
Copy link
Collaborator

@kswang1029 kswang1029 left a comment

Choose a reason for hiding this comment

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

When we have a spectral profile plot, if we switch the x-axis to "channel", we cannot click a channel to switch to the channel in the image viewer. We cannot zoom/pan the spectral profile plot neither.

@kswang1029 kswang1029 added awaiting code changes For pull requests that require code changes and removed awaiting code review For pull requests that require code review awaiting testing For pull requests that require testing labels May 15, 2024
@YuHsuan-Hwang
Copy link
Collaborator Author

When we have a spectral profile plot, if we switch the x-axis to "channel", we cannot click a channel to switch to the channel in the image viewer. We cannot zoom/pan the spectral profile plot neither.

This is fixed.

@YuHsuan-Hwang YuHsuan-Hwang added awaiting code review For pull requests that require code review awaiting testing For pull requests that require testing and removed awaiting code changes For pull requests that require code changes labels May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting code review For pull requests that require code review awaiting testing For pull requests that require testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants