Skip to content

Deprecate record.database_letters and record.query_letters - #5314

Open
mayuriphad wants to merge 2 commits into
biopython:masterfrom
mayuriphad:fix-1000-deprecate-database-letters
Open

Deprecate record.database_letters and record.query_letters#5314
mayuriphad wants to merge 2 commits into
biopython:masterfrom
mayuriphad:fix-1000-deprecate-database-letters

Conversation

@mayuriphad

@mayuriphad mayuriphad commented Sep 13, 2026 •
edited
Loading

Copy link
Copy Markdown

  • 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.rst file, have run pre-commit
    locally, 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.rst and CONTRIB.rst as part of this pull request, am listed
    already, or do not wish to be listed. (This acknowledgement is optional.)

Fixes #1000. Replaces internal usage with query_length and database_length, and adds deprecation properties for backward compatibility.

Copilot AI lite review requested due to automatic review settings September 13, 2026 03:13
Copilot AI reviewed Sep 13, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@peterjc

peterjc commented Sep 13, 2026

Copy link
Copy Markdown
Member

Please read https://github.com/biopython/biopython/blob/master/CONTRIBUTING.rst and especially the clarification in #5229 that AI tools should not be used on the 'good first issue' items (with broader policy under discussion on #5241).

You have not used the pull request template either...

[Does anyone know which settings Copilot is referring to as I didn't setup automatic reviews?]

@mdehoon

mdehoon commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Note that there is a newer parser for Blast XML files in Bio.Blast. It may not be worth the trouble to fix issues in the older parser in Bio.Blast.NCBIXML.

@peterjc peterjc mentioned this pull request Sep 14, 2026
Merged
3 tasks
@mayuriphad

mayuriphad commented Sep 15, 2026

Copy link
Copy Markdown
Author

Apologies for missing the pull request template! I've updated the description to include it. I've also noted the policy regarding 'good first issue' items and AI tools for future reference. Thank you for pointing that out!

@peterjc

peterjc commented Sep 15, 2026

Copy link
Copy Markdown
Member

If you follow the CONTRIBUTING instructions to run pre-commit locally you'll have already seen the issues flagged by ruff and black:

ruff check...............................................................Failed
- hook id: ruff-check
- exit code: 1

D102 Missing docstring in public method
  --> Bio/Blast/NCBIXML.py:86:9
   |
85 |     @property
86 |     def query_letters(self):
   |         ^^^^^^^^^^^^^
87 |         warnings.warn(
88 |             "query_letters is deprecated; please use query_length instead.",
   |

F821 Undefined name `BiopythonDeprecationWarning`
  --> Bio/Blast/NCBIXML.py:89:13
   |
87 |         warnings.warn(
88 |             "query_letters is deprecated; please use query_length instead.",
89 |             BiopythonDeprecationWarning,
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
90 |             stacklevel=2,
91 |         )
   |

F821 Undefined name `BiopythonDeprecationWarning`
   --> Bio/Blast/NCBIXML.py:98:13
    |
 96 |         warnings.warn(
 97 |             "query_letters is deprecated; please use query_length instead.",
 98 |             BiopythonDeprecationWarning,
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
 99 |             stacklevel=2,
100 |         )
    |

D102 Missing docstring in public method
   --> Bio/Blast/NCBIXML.py:104:9
    |
103 |     @property
104 |     def database_letters(self):
    |         ^^^^^^^^^^^^^^^^
105 |         warnings.warn(
106 |             "database_letters is deprecated; please use database_length instead.",
    |

F821 Undefined name `BiopythonDeprecationWarning`
   --> Bio/Blast/NCBIXML.py:107:13
    |
105 |         warnings.warn(
106 |             "database_letters is deprecated; please use database_length instead.",
107 |             BiopythonDeprecationWarning,
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
108 |             stacklevel=2,
109 |         )
    |

F821 Undefined name `BiopythonDeprecationWarning`
   --> Bio/Blast/NCBIXML.py:116:13
    |
114 |         warnings.warn(
115 |             "database_letters is deprecated; please use database_length instead.",
116 |             BiopythonDeprecationWarning,
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
117 |             stacklevel=2,
118 |         )
    |

Found 6 errors.

black....................................................................Failed
- hook id: black
- exit code: 1

would reformat Bio/Blast/NCBIXML.py
would reformat Tests/test_NCBI_qblast.py

Oh no! 💥 💔 💥
2 files would be reformatted, 545 files would be left unchanged.

peterjc
peterjc reviewed Sep 15, 2026
View reviewed changes
Comment thread Bio/Blast/NCBIXML.py
def query_letters(self):
warnings.warn(
"query_letters is deprecated; please use query_length instead.",
BiopythonDeprecationWarning,

@peterjc peterjc Sep 15, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You need to add from Bio import BiopythonDeprecationWarning near the top of the file (where BiopythonParserWarning is already being imported).

@peterjc peterjc Sep 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Import this once at the top of the file please, about line 37 (where BiopythonParserWarning is already being imported).

@peterjc peterjc mentioned this pull request Sep 15, 2026
Closed
@peterjc

peterjc commented Sep 16, 2026

Copy link
Copy Markdown
Member

Could you confirm this was not done with AI please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peterjc peterjc peterjc left review comments
Copilot code review Copilot
Copilot review effort, defaults to Lite
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Deprecate record.database_letters, record.query_letters

4 participants

@mayuriphad @peterjc @mdehoon