Conversation
Codecov Report
@@ Coverage Diff @@
## golden #39 +/- ##
===========================================
- Coverage 100.00% 95.29% -4.71%
===========================================
Files 15 15
Lines 567 723 +156
Branches 71 96 +25
===========================================
+ Hits 567 689 +122
- Misses 0 21 +21
- Partials 0 13 +13
Continue to review full report at Codecov.
|
| return Fingerprint.from_string(self.key or "") | ||
|
|
||
| @property | ||
| def id(self) -> int64: |
There was a problem hiding this comment.
What's the difference (in purpose) between id and key?
| return f.key or int64(0) | ||
|
|
||
| def load(self, graph_id: int64) -> Graph: | ||
| if not self.backend: |
There was a problem hiding this comment.
I think I get that each graph belongs to a single backend, and a given backend can/should hold multiple graphs, but do we like this as a design choice? I.e., "a graph has a backend"? It feels like the more natural relationship is that a graph (or maybe I mean a "web of artifacts") can/should exist on its own during "declaration", and a backend is only necessary during either "read" or "write"?
|
Gonna close since things have significantly changed (particularly using |
3 participants
Adds:
One caveat is that the current Artifact reading/writing functionality depends on the schema, format, and storage being instance-level attributes rather than class-level, e.g.
Artifact(schema=...).to_dict()anda = Artifact.from_dict({"schema": {...}}). Reading through the comments in the Artifact class again though, it looks like some of these aren't meant to be set by the instance.