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.

Sign up for GitHub

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

Jump to bottom

EVA-3525 update validation report #39

Merged
merged 3 commits into from
Jun 4, 2024
Merged

EVA-3525 update validation report #39

merged 3 commits into from
Jun 4, 2024

Conversation

nitin-ebi
Copy link
Collaborator

  1. Warning counts removed from VCF Validtion:
    warning removed
  2. Sample Name check - section heading update
    Sample section heading update
  3. Project section added with files mapping
    project section with files mapping
  4. Gaps increased between 2 collapsible sections and also between collapsible section and the next section
    Also added icon for showing list is expandable
    increased gap

@nitin-ebi nitin-ebi requested review from tcezard and apriltuesday May 30, 2024 00:01
@nitin-ebi nitin-ebi self-assigned this May 30, 2024
apriltuesday
apriltuesday reviewed May 30, 2024
View reviewed changes
Copy link
Contributor

@apriltuesday apriltuesday left a comment

Choose a reason for hiding this comment

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

Some implementation suggestions, the HTML report looks way better though 👍

eva_sub_cli/jinja_templates/sample_name_check.html Outdated Show resolved Hide resolved
eva_sub_cli/jinja_templates/sample_name_check.html Outdated Show resolved Hide resolved
eva_sub_cli/validators/validator.py Show resolved Hide resolved
eva_sub_cli/jinja_templates/fasta_check.html Outdated Show resolved Hide resolved
eva_sub_cli/jinja_templates/project_details.html Outdated Show resolved Hide resolved
eva_sub_cli/validators/validator.py
except Exception as e:
self.error('Error building Validation Report : Error getting info from metadata file' + str(e))
else:
self.error('Error building validation report : Metadata file not present')
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this does almost exactly the same thing as this logic in the fasta check. It should be possible to either extract and re-use that logic, or adjust the results from that check and then use those directly in the report (currently looks like this - I think it just needs the vcf file listing).

If we're worried about the fasta check not being run but still wanting to include the summary, I guess code reuse is the best bet. But #38 should help make that one more reliable anyway.

apriltuesday
apriltuesday approved these changes Jun 3, 2024
View reviewed changes
Copy link
Contributor

@apriltuesday apriltuesday left a comment

Choose a reason for hiding this comment

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

As discussed, the refactoring comment is optional and can be done later if needed 👍

@nitin-ebi nitin-ebi merged commit d01ee0c into EBIvariation:main Jun 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apriltuesday apriltuesday apriltuesday approved these changes

@tcezard tcezard tcezard approved these changes

Assignees

@nitin-ebi nitin-ebi

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@nitin-ebi @tcezard @apriltuesday