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 public docstring to Bio.Seq #1251

Merged
merged 2 commits into from
May 22, 2017
Merged

Conversation

Gasta88
Copy link
Contributor

@Gasta88 Gasta88 commented May 22, 2017

As mentioned in issue #1203 I've added public docstring to Bio.Seq module.

@codecov
Copy link

codecov bot commented May 22, 2017

Codecov Report

Merging #1251 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1251      +/-   ##
==========================================
- Coverage   79.59%   79.58%   -0.01%     
==========================================
  Files         323      323              
  Lines       51701    51701              
==========================================
- Hits        41149    41147       -2     
- Misses      10552    10554       +2
Impacted Files Coverage Δ
Bio/Seq.py 98.35% <100%> (ø) ⬆️
Bio/NeuralNetwork/Gene/Schema.py 71.07% <0%> (-0.83%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 06e15e6...672897c. Read the comment docs.

Bio/Seq.py Outdated
@@ -62,7 +63,7 @@ def _maketrans(complement_mapping):


class Seq(object):
"""A read-only sequence object (essentially a string with an alphabet).
"""Create a read-only sequence object (essentially a string with an alphabet).
Copy link
Member

Choose a reason for hiding this comment

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

The __init__ could be said to create the object, but that does not make sense for the class docstring. You could just remove the A and use Read-only sequence object (essentially a string with an alphabet).

Bio/Seq.py Outdated
@@ -198,12 +199,12 @@ def __eq__(self, other):
return str(self) == str(other)

def __ne__(self, other):
"""Not equal, see __eq__ documentation."""
"""Compare sequences with a not-equal operand, see __eq__ documentation."""
Copy link
Member

Choose a reason for hiding this comment

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

Is this really clearer (likewise for the other special methods)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about """Implement the XYZ operand.""" without any reference to __eq__?

Copy link
Member

Choose a reason for hiding this comment

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

That seems like an improvement.

Bio/Seq.py Outdated
@@ -513,7 +514,7 @@ def rfind(self, sub, start=0, end=sys.maxsize):
return str(self).rfind(sub_str, start, end)

def startswith(self, prefix, start=0, end=sys.maxsize):
"""Does the Seq start with the given prefix? Returns True/False.
"""Reurn True if the Seq starts with the given prefix, False otherwise.
Copy link
Member

Choose a reason for hiding this comment

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

Typo (Return)

Bio/Seq.py Outdated
@@ -544,7 +545,7 @@ def startswith(self, prefix, start=0, end=sys.maxsize):
return str(self).startswith(prefix_str, start, end)

def endswith(self, suffix, start=0, end=sys.maxsize):
"""Does the Seq end with the given suffix? Returns True/False.
"""Return True if the Seq end with the given suffix, False otherwise.
Copy link
Member

Choose a reason for hiding this comment

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

end --> ends

Bio/Seq.py Outdated
@@ -1139,7 +1140,7 @@ def ungap(self, gap=None):


class UnknownSeq(Seq):
"""A read-only sequence object of known length but unknown contents.
"""Create a read-only sequence object of known length but unknown contents.
Copy link
Member

Choose a reason for hiding this comment

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

See comment for the Seq class docstring, Create ... is not quite right here.

Bio/Seq.py Outdated
@@ -1391,7 +1394,7 @@ def complement(self):
return self

def reverse_complement(self):
"""The reverse complement of an unknown nucleotide equals itself.
"""Return the reverse complement of an unknown nucleotide equals itself.
Copy link
Member

Choose a reason for hiding this comment

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

The new version does not make sense to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The function itself does not make sense.

How about Return the reverse complement of an unknown nucleotide,which equals itself.?

Copy link
Member

Choose a reason for hiding this comment

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

If you think the function does not make sense, then clearly the current docstring is not clear enough. How about:

        def reverse_complement(self):
            """Return the reverse complement of an unknown sequence.

            The reverse complement of an unknown nucleotide equals itself:

            >>> from Bio.Seq import UnknownSeq
            >>> from Bio.Alphabet import generic_dna
            >>> example = UnknownSeq(6, generic_dna)
            >>> print(example)
            NNNNNN
            >>> print(example.reverse_complement())
            NNNNNN
            """

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Much better, thanks.

Bio/Seq.py Outdated
@@ -1556,7 +1559,7 @@ def ungap(self, gap=None):


class MutableSeq(object):
"""An editable sequence object (with an alphabet).
"""Create an editable sequence object (with an alphabet).
Copy link
Member

Choose a reason for hiding this comment

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

Again, Create ... is not appropriate.

Bio/Seq.py Outdated
@@ -1986,7 +2001,7 @@ def transcribe(dna):


def back_transcribe(rna):
"""Back-transcribes an RNA sequence into DNA.
"""Return the RNA back-transcribed sequence into DNA.
Copy link
Member

Choose a reason for hiding this comment

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

How about Return the RNA sequence back-transcribed into DNA.

Bio/Seq.py Outdated
@@ -2009,7 +2024,7 @@ def back_transcribe(rna):

def _translate_str(sequence, table, stop_symbol="*", to_stop=False,
cds=False, pos_stop="X", gap=None):
"""Helper function to translate a nucleotide string (PRIVATE).
"""Translate a nucleotide to string (PRIVATE).
Copy link
Member

Choose a reason for hiding this comment

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

"a nucleotide" implies a single base letter.

How about Translate nucleotide string into a protein string (PRIVATE).

@Gasta88
Copy link
Contributor Author

Gasta88 commented May 22, 2017

File edited.


>>> my_nuc = UnknownSeq(10)
>>> my_nuc
Copy link
Member

Choose a reason for hiding this comment

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

Is there a tab here? The display on GitHub looks different for some reason...

Copy link
Member

Choose a reason for hiding this comment

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

Well, if it was a tab it is being removed in this change so never mind.

@peterjc peterjc merged commit 9129c0f into biopython:master May 22, 2017
peterjc pushed a commit to peterjc/biopython that referenced this pull request May 23, 2017
MarkusPiotrowski pushed a commit to MarkusPiotrowski/biopython that referenced this pull request Oct 31, 2017
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