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 dunder methods to the Python Record class #91

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

apcamargo
Copy link
Contributor

@apcamargo apcamargo commented Dec 19, 2024

This PR introduces several dunder methods to the Python Record class:

  • __new__: Constructor that allows the direct creation of Record objects:
>>> fasta_record = needletail.Record("SRR1749083.HWI-ST1309F", "NGACTCGTC")
>>> fastq_record = needletail.Record("SRR1749083.HWI-ST1309F", "NGACTCGTC", "#<<BBF/FF")
  • __hash__: Returns an integer hash based on the record's ID, sequence, and quality (if available). This overrides the default __hash__, which generates different hashes for identical objects.
  • __eq__: Compares two Record objects by checking if their IDs, sequences, and qualities (if available) are identical. While we could check for equality based on hashes, this would involve additional computation.
  • __len__: Returns the length of the sequence, similar to BioPython's behavior.
  • __str__: Returns a string formatted in FASTA/FASTQ style to facilitate writing FASTX files. Currently, sequences are wrapped at 60 characters for FASTA records, though the wrapping can be adjusted.
  • __repr__: Provides a simple string representation that includes the record ID, a sequence snippet, and the quality string (when available):
>>> needletail.Record("SRR1749083.HWI-ST1309F", "NGACTCGTC")
# Record(id=SRR1749083.HWI-ST1309F, sequence=NGACTCGTC, quality=None)

Additionally, is_fasta and is_fastq have been converted to properties.

Test coverage for these methods has not been added yet, but I will implement it if this PR is approved.

@apcamargo apcamargo changed the title Add dunder methods for the Python Record class Add dunder methods to the Python Record class Dec 19, 2024
src/python.rs Outdated Show resolved Hide resolved
src/python.rs Outdated Show resolved Hide resolved
src/python.rs Show resolved Hide resolved
@audy
Copy link
Contributor

audy commented Jan 22, 2025

@apcamargo thank you very much! I left some comments. Otherwise, these changes look like great additions to the library and we'd be happy to merge them in.

@apcamargo
Copy link
Contributor Author

Thanks for the review, @audy! I just pushed commits that address all your comments.

I prefer wrapping since that's how other libraries/tools format FASTAs, and I think users expect it. That said, I’m not too attached to it :)

@apcamargo apcamargo requested a review from audy January 22, 2025 20:56
@audy
Copy link
Contributor

audy commented Jan 22, 2025

@apcamargo thanks for making those changes. Let me know if you're good to merge.

@apcamargo
Copy link
Contributor Author

apcamargo commented Jan 23, 2025

@audy I’ve added docstrings to all classes, methods, and functions. The only potential change I’d consider is adding a default value to the iupac parameter in both normalize_seq and normalize.

@apcamargo
Copy link
Contributor Author

apcamargo commented Jan 24, 2025

@audy I also updated parse_fastx_file to make it able to take pathlib.Path objects, in addition to str.

@audy
Copy link
Contributor

audy commented Jan 24, 2025

@apcamargo Awesome! Do you mind adding some tests?

The only potential change I’d consider is adding a default value to the iupac parameter in both normalize_seq and normalize.

I support this change but I'm not sure if it should default to True or False. Thoughts?

@apcamargo
Copy link
Contributor Author

Apologies, @audy! I had forgotten about the tests.

I've now added tests for all the new functions and methods, and set the default value of iupac to False.

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