inconsistent type annotations for Document.on_snapshot

[igor47]

Looking here (which mirrors the API docs):

python-firestore/google/cloud/firestore_v1/document.py

Lines 503 to 504 in de3aca0

	callback(Callable[[:class:`~google.cloud.firestore.document.DocumentSnapshot`], NoneType]):
	a callback to run when a change occurs

The claim is that the callback invoked by on_snapshot should have type signature Callable[[:class:~google.cloud.firestore.document.DocumentSnapshot], NoneType]. however, just a few lines lower, in the example, it looks like the callback receives three arguments:

python-firestore/google/cloud/firestore_v1/document.py

Line 515 in de3aca0

def on_snapshot(document_snapshot, changes, read_time):

i tried to understand what's going on by reading the code, and didn't make too much progress. we invoke the callback here:

python-firestore/google/cloud/firestore_v1/watch.py

Lines 569 to 573 in de3aca0

	self._snapshot_callback(
	keys,
	appliedChanges,
	datetime.datetime.fromtimestamp(read_time.seconds, pytz.utc),
	)

it looks like there are indeed three arguments passed. appliedChanges appears to be a List[DocumentChange], and the final argument seems to be a datetime.datetime instance. but the first argument... is the result of calling keys() on a document_tree (here:

python-firestore/google/cloud/firestore_v1/watch.py

Line 567 in de3aca0

keys = sorted(updated_tree.keys(), key=key)

) and so maybe is a List[str] ? the self.doc_tree is an instance of WatchDocTree and it's keys() method returns the underlying dict's keys. are those keys actually DocumentSnapshot instances? it maybe sort of seems like it, after tracing through the code a bunch. i'm guessing that the correct type signature for the callback is:

Callable[List[DocumentSnapshot], List[DocumentChange], datetime.datime], None]

but it would be nice if this was confirmed, and the documentation updated to reflect reality.

[HemangChothani]

Closing as resolved by #79 PR