Skip to content

Conversation

@ple13
Copy link
Contributor

@ple13 ple13 commented Dec 17, 2025

This PR closes #3352.

@wfa-reviewable
Copy link

This change is Reviewable

Copy link
Contributor

@tristanvuong2021 tristanvuong2021 left a comment

Choose a reason for hiding this comment

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

@tristanvuong2021 reviewed all commit messages and made 1 comment.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @kungfucraig and @ple13).


src/main/python/wfa/measurement/reporting/postprocessing/report/report.py line 483 at r1 (raw file):

        for edp in self._weekly_non_cumulative_measurements.keys()
        if len(self._weekly_non_cumulative_measurements[edp]) > 0 and
          self._weekly_non_cumulative_measurements[edp][0].k_reach

Does this fix need to be applied in other places. I only tested reach so far

Copy link
Member

@kungfucraig kungfucraig left a comment

Choose a reason for hiding this comment

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

@kungfucraig reviewed 2 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tristanvuong2021).


src/main/python/wfa/measurement/reporting/postprocessing/report/report.py line 483 at r1 (raw file):

Previously, tristanvuong2021 (Tristan Vuong) wrote…

Does this fix need to be applied in other places. I only tested reach so far

+1

Copy link
Contributor Author

@ple13 ple13 left a comment

Choose a reason for hiding this comment

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

@ple13 made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @tristanvuong2021).


src/main/python/wfa/measurement/reporting/postprocessing/report/report.py line 483 at r1 (raw file):

Previously, kungfucraig (Craig Wright) wrote…

+1

Yeah, this is the only place that caused the issue. It returns a wrong set of edp combinations for k-reach.

Copy link
Contributor

@tristanvuong2021 tristanvuong2021 left a comment

Choose a reason for hiding this comment

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

@tristanvuong2021 resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @stevenwarejones).

Copy link
Contributor

@tristanvuong2021 tristanvuong2021 left a comment

Choose a reason for hiding this comment

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

@tristanvuong2021 reviewed 2 files.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @stevenwarejones).

Copy link
Collaborator

@stevenwarejones stevenwarejones left a comment

Choose a reason for hiding this comment

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

@stevenwarejones reviewed 2 files and all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @ple13).

@ple13 ple13 merged commit d63220e into main Dec 18, 2025
5 of 6 checks passed
@ple13 ple13 deleted the lephi-k-reach-empty branch December 18, 2025 23:23
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.

The report processor adds wrong equal constraints between reach and an empty k_reach.

6 participants