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

Minor refactoring of polarization values and defaults in the slicer function #1390

Merged
merged 4 commits into from
Nov 6, 2024

Conversation

confluence
Copy link
Collaborator

@confluence confluence commented Nov 1, 2024

Description

This is a set of refactoring changes:

  • Removing redundant lookups of integers with same integer enums
  • Using correct protobuf function to cast integers to integer enums
  • General cleanup of polarization values (descriptions; CASA and FITS conversion...)
  • Replacing unnecessarily complex and inefficient ordering of small unordered maps with polarization keys (these can just be ordered maps)
  • Precalculating fixed ranges for full X, Y and Z and using the X and Y ranges as defaults to GetImageSlicer instead of using a placeholder default which has to be checked and possibly replaced whenever the function is called
  • Reimplementing functions for mapping polarization index to polarization value and vice versa

Checklist

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

…ps; consolidated some polarization functions into one utility class; eliminated two unnecessary placeholder range checks from frame slicer code; eliminated several unnecessary linear searches through map keys by replacing small unordered maps with ordered maps and by adding a reverse lookup to file loader
Copy link

github-actions bot commented Nov 1, 2024

Code Coverage

Package Line Rate Health
src.Cache 72%
src.DataStream 44%
src.FileList 67%
src.Frame 35%
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 37%
Summary 46% (8634 / 18964)

Copy link
Collaborator

@pford pford left a comment

Choose a reason for hiding this comment

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

I did not realize the protobuf enums had these built-in functions. We should check if this can be used in other refactoring!

Copy link
Contributor

@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.

all e2e tests are passing. regular Stokes cubes are fine (including hypercubes). rotated cubes are fine in switching IQUV (will crash if switching computed components).

@confluence confluence merged commit a2834cd into dev Nov 6, 2024
14 checks passed
@kswang1029 kswang1029 added the awaiting merge For pull requests ready for merge or pending frontend/protobuf changes label Nov 6, 2024
@confluence confluence deleted the confluence/stokes-and-range-refactor branch November 6, 2024 13:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting merge For pull requests ready for merge or pending frontend/protobuf changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants