Skip to content

Conversation

r-xue
Copy link
Owner

@r-xue r-xue commented Oct 13, 2025

No description provided.

@Copilot Copilot AI review requested due to automatic review settings October 13, 2025 13:22
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors duplicate removal operations across multiple files by introducing and using a centralized deduplicate utility function. The changes improve code consistency and maintainability by replacing various manual deduplication patterns with a standardized approach.

  • Introduces a new deduplicate utility function from pipeline.infrastructure.utils
  • Replaces manual set-based deduplication with the new utility function
  • Removes unused imports and warning suppressions

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pipeline/hifa/tasks/applycal/ampphase_vs_freq_qa.py Removes unused warnings import and warning suppression code
pipeline/hifa/heuristics/wvrgcal.py Refactors tie calculation to use deduplicate utility
pipeline/hif/tasks/setmodel/setmodel.py Replaces set comprehensions with deduplicate utility calls
pipeline/hif/tasks/fluxscale/fluxscale.py Replaces set comprehensions with deduplicate utility calls
pipeline/h/tasks/common/viewflaggers.py Consolidates imports and replaces list(set()) with deduplicate
pipeline/h/heuristics/fieldnames.py Adds utils import and uses deduplicate for field processing

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

reference_fields = field_fn.calculate(self.ms, self.refintent)
# run the answer through a set, just in case there are duplicates
fields = {s for s in utils.safe_split(reference_fields)}
fields = utils.deduplicate(s for s in utils.safe_split(reference_fields))
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

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

The deduplicate function is being called with a generator expression. Verify that the deduplicate function can handle generators properly, as some deduplication implementations expect sequences or iterables that can be consumed multiple times.

Suggested change
fields = utils.deduplicate(s for s in utils.safe_split(reference_fields))
fields = utils.deduplicate(list(utils.safe_split(reference_fields)))

Copilot uses AI. Check for mistakes.

# run the answer through a set, just in case there are duplicates
fields = {s for s in utils.safe_split(reference_fields)}

fields = utils.deduplicate(s for s in utils.safe_split(reference_fields))
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

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

The deduplicate function is being called with a generator expression. Verify that the deduplicate function can handle generators properly, as some deduplication implementations expect sequences or iterables that can be consumed multiple times.

Suggested change
fields = utils.deduplicate(s for s in utils.safe_split(reference_fields))
fields = utils.deduplicate(list(s for s in utils.safe_split(reference_fields)))

Copilot uses AI. Check for mistakes.

scans = ms.get_scans(scan_intent=intent, spw=spw)
fields = {fld for scan in scans for fld in scan.fields}

fields = utils.deduplicate(fld for scan in scans for fld in scan.fields)
Copy link

Copilot AI Oct 13, 2025

Choose a reason for hiding this comment

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

The deduplicate function is being called with a generator expression. Verify that the deduplicate function can handle generators properly, as some deduplication implementations expect sequences or iterables that can be consumed multiple times.

Suggested change
fields = utils.deduplicate(fld for scan in scans for fld in scan.fields)
fields = utils.deduplicate(list(fld for scan in scans for fld in scan.fields))

Copilot uses AI. Check for mistakes.

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.

1 participant