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

Dev gcgi 1481 raw coverage autofill #504

Merged
merged 6 commits into from
Jan 9, 2025

Conversation

OumaimaHamza
Copy link
Collaborator

No description provided.

@OumaimaHamza OumaimaHamza requested a review from iainrb January 7, 2025 21:51
Copy link
Collaborator

@iainrb iainrb left a comment

Choose a reason for hiding this comment

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

Looks great! A few very minor fixes and then we should be good to go.

# Check if coverage values are unique
coverage = filtered_data[columns_of_interest.MeanBaitCoverage].unique()
if len(coverage) != 1:
msg = f"Multiple coverage values found for group_id {group_id}: {coverage}."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please log the error with self.logger.error(msg) before calling raise

selected_value = coverage[0]
qc_dict[constants.RAW_COVERAGE] = int(round(selected_value, 0))
else:
msg = f"No valid QC metrics found for group_id {group_id} after filtering out the normal."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please log the error with self.logger.error(msg) before calling raise

@@ -125,22 +150,22 @@ def render(self, data):
def process_ichor_json(self, ichor_metrics):
with open(ichor_metrics, 'r') as ichor_results:
ichor_json = json.load(ichor_results)
return(ichor_json)
return (ichor_json)
Copy link
Collaborator

Choose a reason for hiding this comment

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

No parentheses needed here -- return ichor_json works and is simpler

header_line = False
else:
next
return(int(round(unique_coverage, 0)))
return (int(round(unique_coverage, 0)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Again, outermost parentheses are not needed -- can have return int(round(unique_coverage, 0))

@OumaimaHamza OumaimaHamza requested a review from iainrb January 8, 2025 14:30
Copy link
Collaborator

@iainrb iainrb left a comment

Choose a reason for hiding this comment

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

Looks great!

@OumaimaHamza OumaimaHamza merged commit dea9e98 into release-1.7.9 Jan 9, 2025
1 check passed
@OumaimaHamza OumaimaHamza deleted the dev-GCGI-1481_raw-coverage-autofill branch January 9, 2025 14:35
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.

2 participants