Skip to content

add diann file support and tests#24

Merged
ltatka merged 6 commits intomainfrom
feat/read_diann
Mar 26, 2025
Merged

add diann file support and tests#24
ltatka merged 6 commits intomainfrom
feat/read_diann

Conversation

@ltatka
Copy link
Copy Markdown
Member

@ltatka ltatka commented Mar 24, 2025

No description provided.

@ltatka ltatka requested review from jspaezp and wfondrie March 24, 2025 20:57
)
return proteins

def read_diann(proteins_tsv: str) -> pd.DataFrame:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion: rename to read_diann_pg_mat or something that denotes which file is supported

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I have no objections to this. The reason I did it this way was to be consistent with the current parser function names, which are read_encyclopedia and read_metamorpheus. Do you think those function names should also be updated?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I feel like the difference there is that (as far as I can recall ...) those guys only return one matrix (I might be wrong though)

@ltatka ltatka merged commit f4566fd into main Mar 26, 2025
0 of 2 checks passed
)

# Check data types
# (if loading from S3, default types are 'O'
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can pandas read from s3?


# Check data types
# (if loading from S3, default types are 'O'
if proteins.index.dtype not in ["O", "category", "str"]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

why did you go with this instead of trying to cast to a number and fail if it cannot?

@jspaezp jspaezp mentioned this pull request Mar 27, 2025
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