refactor: extract common logic to TableBase class #956
Conversation
product-auto-label
bot
added
the
api: bigquery
google-cla
bot
added
the
cla: yes
google/cloud/bigquery/table.py
Outdated
| @@ -126,45 +127,93 @@ def _view_use_legacy_sql_getter(table): | |||
| return True | |||
|
|
|||
|
|
|||
| class TableReference(object): | |||
| """TableReferences are pointers to tables. | |||
| class TableBase: | |||
There was a problem hiding this comment.
Let's either make this private (_TableBase) or document it + update relevant types in the client module.
There was a problem hiding this comment.
I don't see the end users using TableBase directly, it just serves as a central place where we keep the common functionality. An implementation detail, in other words.
Let's mark it as private.
There was a problem hiding this comment.
The contrarian point is that parameter types might use a public TableBase to signal the minimal contract that their arguments require, which is more focused than Union[Table, TableRef, ...]. I don't see any case where TableBase as a return type make sense.
There was a problem hiding this comment.
After some thought I'm leaning towards keeping the base class private if the main reason for making it public is cleaner type annotations. I view type annotations more as an optional helper, and not as an end goal that would require adjusting the code just for the annotations' sake. As a bonus we still have an option to define our own TableType alias if we don't like Union[Table, ...].
Or does anybody have a strong contrary opinion here?
plamut
added
the
automerge
gcf-merge-on-green
bot
removed
the
automerge
* refactor: extract common logic to TableBase class * Adress pytype's missing attribute false warning * Mark TableBase class as private * Simplify TableReference.to_api_repr() logic * Avoid get/set subproperty helper gotcha * Test _TableBase class directly
Closes #871.
PR checklist: