fix: Deprecate query_letters and database_letters attributes#5168
fix: Deprecate query_letters and database_letters attributes#5168A404coder wants to merge 6 commits intobiopython:masterfrom
Conversation
…ibutes - Convert query_letters and database_letters to properties with deprecation warnings - Maintain backward compatibility - attributes still work but emit warnings - Direct users to use query_length and database_length instead - Addresses issue biopython#1000 opened in 2016
|
This is what I expected on the code side, which is good. You've ignored the pull request template - please address There should be a test accessing the Otherwise, please update the tests to silence the new Also please add an entry to the |
|
Thanks for the detailed review! I've addressed the feedback in the latest commit:
I'll also fill in the PR template shortly. All tests pass locally. |
…th, add DEPRECATED entry - Use _query_letters internally in parser to avoid triggering deprecation warnings during XML parsing - Replace query_letters with query_length in test_NCBIXML.py and test_NCBI_qblast.py - Add tests confirming BiopythonParserWarning for deprecated query_letters and database_letters - Add Bio.Blast.NCBIXML entry to DEPRECATED.rst
7384662 to
d639795
Compare
| # query_length properties (as in the plain text parser, see | ||
| # Bug 2176 comment 12): | ||
| self._blast.query_length = self._blast.query_letters | ||
| self._blast.query_length = self._blast._query_letters |
There was a problem hiding this comment.
You might update the comment just below this?
There was a problem hiding this comment.
Good catch! Updated the comment in the latest commit — removed the old "hack" / "transition period" wording and replaced it with a note that query_letters is now deprecated in favour of query_length.
|
By eye this looks good - something off in the formatting, black reports: |
Fix formatting issues flagged by CI (black reports would reformat test_NCBIXML.py and test_NCBI_qblast.py).
Address review comment from peterjc - the old comment mentioned recording query length as both query_letters and query_length with a future deprecation plan. Since query_letters is now deprecated, update the comment to reflect the current state.
|
Thanks for catching the formatting issue! I've run black on both test files and also updated the outdated comment about the query_letters/query_length transition (as you suggested in the inline review). All should be clean now. |
|
Ah: "Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com" We (Biopython) don't yet have a formal policy about this yet, but that is becoming more pressing. See also my blog post from late last year: https://blastedbio.blogspot.com/2025/11/thoughts-on-generative-ai-contributions.html I was just involved in a conversation earlier today that "Good first issue" tagged issues (or their equivalents) ought to be explicitly reserved for humans, and not AI agents. Most projects (Biopython too) have deliberately left these "low hanging fruit" which the maintainers could easily do themselves, as real-world tasks to train beginners (even though this will often cost the maintainers more time). This is as an investment for the future (those people are then more likely to make further more valuable contribution here or elsewhere). At least in this case the scope and nature of the changes is such that we don't need to worry about copyright issues. |
The TODO has been addressed in this PR — database_letters is now deprecated. Remove the stale comment as suggested by reviewer.
632fbf1 to
313f625
Compare
I hereby agree to dual licence this and any previous contributions under both
the Biopython License Agreement AND the BSD 3-Clause License.
I have read the
CONTRIBUTING.rstfile, have runpre-commitlocally, and understand that continuous integration checks will be used to
confirm the Biopython unit tests and style checks pass with these changes.
I have added my name to the alphabetical contributors listings in the files
NEWS.rstandCONTRIB.rstas part of this pull request, am listedalready, or do not wish to be listed. (This acknowledgement is optional.)
Closes #1000
Changes
query_lettersanddatabase_lettersto properties inHeaderclass with deprecation warnings viaBiopythonParserWarning_query_lettersdirectly in parser internals to avoid triggering warnings during XML parsingtest_NCBIXML.pyandtest_NCBI_qblast.pyto usequery_lengthinstead ofquery_lettersquery_lettersanddatabase_lettersDEPRECATED.rstNEWS.rstandCONTRIB.rstContext
The issue has been open since 2016. The XML parser sets both
query_lettersandquery_length, as well asdatabase_lettersanddatabase_length. This change helps users migrate to the preferred attributes while maintaining backward compatibility.Testing
query_lengthreplacingquery_lettersBiopythonParserWarningis raised when accessing deprecated attributes