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

Add a small system test for sage2lib #9

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

wfondrie
Copy link
Member

This PR adds a very small test for sage2lib.

Copy link

codecov bot commented Jun 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.86%. Comparing base (40dfebd) to head (29da124).

Additional details and impacted files
@@             Coverage Diff             @@
##             main       #9       +/-   ##
===========================================
+ Coverage   43.20%   60.86%   +17.66%     
===========================================
  Files           8        8               
  Lines         250      253        +3     
===========================================
+ Hits          108      154       +46     
+ Misses        142       99       -43     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wfondrie
Copy link
Member Author

Hmm... the Windows error here seems to originate from Pyteomics within ms2ml 🤔

Do we actually care about supporting Windows here? Probably not.

@wfondrie wfondrie assigned wfondrie and jspaezp and unassigned wfondrie and jspaezp Jun 26, 2024
@wfondrie wfondrie requested a review from jspaezp June 27, 2024 16:00
@@ -24,7 +24,11 @@ def read_peptides(peptides, qvalue):
polars.DataFrame
The parsed and filtered peptides.
"""
peptide_df = pl.read_csv(peptides, separator="\t")
try:
Copy link

Choose a reason for hiding this comment

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

Request: Could we add an error/warning message saying the fallback is occurring?

peptide_df = pl.read_csv(peptides, separator="\t")
try:
peptide_df = pl.read_parquet(peptides)
except pl.exceptions.ComputeError:
Copy link

Choose a reason for hiding this comment

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

R: Can we add a check of the error to make sure the compute error comes from the parsing and not anywhere else?

import pytest


@pytest.mark.parametrize("ftype", ["parquet", "csv"])
Copy link

Choose a reason for hiding this comment

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

Note: Since the whole XZ situation ive become a bit more paranoid ... we whould make a PR so the .parquet is generated from a .csv ... just to have no binaries in source control.

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