Skip to content

Comments

Int as a view, not Python#66

Merged
JacobHayes merged 7 commits intogoldenfrom
simple_producer_iterations
Jul 26, 2021
Merged

Int as a view, not Python#66
JacobHayes merged 7 commits intogoldenfrom
simple_producer_iterations

Conversation

@mikss
Copy link
Contributor

@mikss mikss commented Jul 23, 2021

After some discussion about Views, we came to some level of agreement that Views should encode “structural” metadata about the object’s form (e.g., int), rather than “environmental” metadata about where the object is being worked on (e.g., via Python interpreter). Note that BigQuery is a confusing case, because a BigQuery table is both a form in which data is manipulated, and BigQuery is also the managed database service through which tables are manipulated.

Given the above, this PR refactors the example view to be an Int, rather than Python (although this remains as a helpful superclass in the renamed form _PythonObject, for ease of general read/write).

@codecov
Copy link

codecov bot commented Jul 23, 2021

Codecov Report

Merging #66 (0cb46f7) into golden (182316f) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            golden       #66   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           22        22           
  Lines          668       679   +11     
  Branches        74        75    +1     
=========================================
+ Hits           668       679   +11     
Impacted Files Coverage Δ
src/arti/internal/utils.py 100.00% <100.00%> (ø)
src/arti/views/core.py 100.00% <100.00%> (ø)
src/arti/views/python.py 100.00% <100.00%> (ø)

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 182316f...0cb46f7. Read the comment docs.

Copy link
Member

@JacobHayes JacobHayes left a comment

Choose a reason for hiding this comment

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

Good improvement, thanks! A few interface Qs, but glad to see the read/write will work so simply.



RegisterK = TypeVar("RegisterK", bound=str)
RegisterK = TypeVar("RegisterK")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is so we can register .python_type as a key.

@mikss mikss requested a review from JacobHayes July 26, 2021 14:32
@mikss mikss marked this pull request as ready for review July 26, 2021 14:32
@mikss mikss requested a review from joycex99 July 26, 2021 14:36
Copy link
Member

@JacobHayes JacobHayes left a comment

Choose a reason for hiding this comment

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

Looks good!

Co-authored-by: Jacob Hayes <[email protected]>
@JacobHayes JacobHayes merged commit 6b7f7e4 into golden Jul 26, 2021
@JacobHayes JacobHayes deleted the simple_producer_iterations branch July 26, 2021 17:46
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