docs: Docstrings for `django_spanner` #564

mf2199 · 2020-12-03T18:35:43Z

No description provided.

…django into docstr-2

google-cla · 2020-12-03T18:35:58Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

mf2199 · 2020-12-03T18:36:15Z

@googlebot I consent.

google-cla · 2020-12-03T18:36:27Z

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

tina80lvl · 2020-12-03T20:37:07Z

@googlebot I consent.

django_spanner/client.py

django_spanner/expressions.py

c24t

The PR looks like a clear improvement (aside from the odd unicode chars), any reason it's still in draft?

c24t · 2020-12-09T07:14:58Z

django_spanner/schema.py

@@ -123,6 +130,13 @@ def create_model(self, model):
                self.create_model(field.remote_field.through)

    def delete_model(self, model):
+        """
+        Drop the model’s table in the database along with any unique constraints


Looks like this is a unicode RIGHT SINGLE QUOTATION MARK, did you write these docstrings in a rich text editor first?

tina80lvl · 2020-12-09T11:31:19Z

The PR looks like a clear improvement (aside from the odd unicode chars), any reason it's still in draft?

I guess there is no such reason for now, I've fixed the rest changes.

c24t

Please fix the TODOs.

c24t · 2020-12-10T22:10:56Z

django_spanner/client.py

    def runshell(self, parameters):
+        """Override the base class method.
+
+        TODO: Consider actual implementation of this method.


It's not clear that we could implement this since cloud spanner doesn't have a "shell" executable. We might want to consider https://github.com/cloudspannerecosystem/spanner-cli or a similar project for this in the future.

I removed the docstring here, just raising NotSupportedError is fine since spanner doesn't really give us a way to support this.

c24t · 2020-12-10T22:15:42Z

django_spanner/operations.py

+        Transform a time value to an object compatible with what is expected
+        by the backend driver for time columns.
+
+        :type value: #TODO


Looks like there are still a lot of TODOs in the docstrings in this file.

c24t · 2020-12-15T20:08:53Z

django_spanner/operations.py

+    # TODO: here and below, remove unused expression and connection parameters
+    # or include them to the code.


@mf2199 I moved this comment in d36b9ab since the preceding change had this comment in each of the converter docstrings:

#TODO remove unused expression and connection parameters or include them to the code.

But converters are part of the django db API, and we can't change the signature without also replacing django internals. See e.g. https://github.com/django/django/blob/8093aaa8ff9dd7386a069c6eb49fcc1c5980c033/django/db/models/sql/compiler.py#L1084.

c24t · 2020-12-15T20:26:58Z

django_spanner/operations.py

    def convert_binaryfield_value(self, value, expression, connection):
+        """Convert a binary field to Cloud Spanner.


AFAICT these docstrings are backwards: value comes from python-spanner, and we're converting from one bytes (or "bytes-like object") it into another: from a base64-encoded bytes returned by spanner to a non-base64-encoded bytes for django, since that's the internal type for django BinaryFields.

c24t · 2020-12-15T20:27:10Z

django_spanner/operations.py

+        """Convert a date and time field to Cloud Spanner.
+
+        :type value: #TODO
+        :param value: A binary field value.


Copy/paste?

tina80lvl and others added 24 commits November 25, 2020 18:32

docs for base

e264a94

docs for client

5069132

docs for compiler

ca9c7ce

docs for expressions

1a6257a

docs for features

cfdba5f

docs for functions

2b742f8

docs for introspection

a82bf1b

resolving conflict

b819547

docs for lookups

321858f

docs for operations

e77b4e9

docs for schema

4db2d4a

docs for utils

115881f

docs: updates to base.py

0647468

docs: updates to client.py

6970b97

docs for validation

8dab421

docs: updates to complier.py

04a0cac

Merge branch 'docstr-2' of https://github.com/q-logic/python-spanner-…

c2a09f0

…django into docstr-2

docs: updates to creation.py

0d2ec73

docs: updates to expressions.py

3a6e84a

docs: updates to features.py

1479a2f

docs: updates to functions.py

c902cd2

docs: updates to introspection.py

6cfb4a5

docs: updates to lookups.py

5580621

docs: updates to operations.py

7deb3d3

product-auto-label bot added the api: spanner Issues related to the googleapis/python-spanner-django API. label Dec 3, 2020

google-cla bot added the cla: no This human has *not* signed the Contributor License Agreement. label Dec 3, 2020

google-cla bot added cla: yes This human has signed the Contributor License Agreement. and removed cla: no This human has *not* signed the Contributor License Agreement. labels Dec 3, 2020

c24t and others added 3 commits December 7, 2020 13:59

Merge branch 'master' into docstr-2

ed86c06

fix: docstrings

67106fa

fix: typo

c9364d0

c24t reviewed Dec 8, 2020

View reviewed changes

django_spanner/client.py Outdated Show resolved Hide resolved

django_spanner/expressions.py Outdated Show resolved Hide resolved

fix: docstrings

678ecb3

c24t reviewed Dec 9, 2020

View reviewed changes

tina80lvl added 2 commits December 9, 2020 14:16

fix: single quotes

0b84fb6

fix: docstrings typing

583398d

mf2199 marked this pull request as ready for review December 10, 2020 15:27

mf2199 requested a review from a team as a code owner December 10, 2020 15:27

c24t added 2 commits December 10, 2020 14:17

Remove runshell docstring

2afc4eb

Remove some TODOs from docstrings

d36b9ab

c24t reviewed Dec 10, 2020

View reviewed changes

Merge branch 'master' into docstr-2

117484b

c24t reviewed Dec 15, 2020

View reviewed changes

c24t and others added 3 commits December 15, 2020 12:38

Fix operations docstrings

5240d21

Merge branch 'master' into docstr-2

499c61a

docs: docstrings

d749356

c24t approved these changes Dec 16, 2020

View reviewed changes

c24t merged commit 7083f1d into googleapis:master Dec 16, 2020

IlyaFaer deleted the docstr-2 branch February 1, 2021 08:09

This was referenced Apr 17, 2024

chore(main): release 3.0.0 #869

Closed

chore(main): release 3.0.0 #875

Closed

chore(main): release 3.0.0 #877

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: Docstrings for `django_spanner` #564

docs: Docstrings for `django_spanner` #564

mf2199 commented Dec 3, 2020

google-cla bot commented Dec 3, 2020

mf2199 commented Dec 3, 2020

google-cla bot commented Dec 3, 2020

tina80lvl commented Dec 3, 2020

c24t left a comment

c24t Dec 9, 2020

tina80lvl Dec 9, 2020

tina80lvl commented Dec 9, 2020

c24t left a comment

c24t Dec 10, 2020

c24t Dec 10, 2020

c24t Dec 10, 2020

c24t Dec 15, 2020

c24t Dec 15, 2020

c24t Dec 15, 2020

		# TODO: here and below, remove unused expression and connection parameters
		# or include them to the code.

		def convert_binaryfield_value(self, value, expression, connection):
		"""Convert a binary field to Cloud Spanner.

docs: Docstrings for django_spanner #564

docs: Docstrings for django_spanner #564

Conversation

mf2199 commented Dec 3, 2020

google-cla bot commented Dec 3, 2020

mf2199 commented Dec 3, 2020

google-cla bot commented Dec 3, 2020

tina80lvl commented Dec 3, 2020

c24t left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tina80lvl commented Dec 9, 2020

c24t left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

docs: Docstrings for `django_spanner` #564

docs: Docstrings for `django_spanner` #564