Add unique ID to Node
#153
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
joscao
force-pushed
the
devel/unique_ids
branch
from
cd1a40e
to
af9bc9d
Compare
See Issue ecmwf-ifs#123, copy follows: Currently, the only mechanism for differentiating two string-identical but distinct nodes is the .source property. However this property is only instantiated for parsed nodes; if two identical but distinct nodes are generated in a transformation there is no method (apart from the object id) to tell them apart. The above could be remedied by adding a uid attribute to the Node class. The uid property would be generated every time a node is created. This means that even copies of the same node would have different values of uid, unless overridden via a constructor argument.
I see no reason why Comments node wouldnt call the __post_init__ in their __post_init__ like all other nodes do.
joscao
force-pushed
the
devel/unique_ids
branch
from
f972093
to
e1b72a1
Compare
Codecov Report
@@ Coverage Diff @@
## main #153 +/- ##
=======================================
Coverage 92.13% 92.13%
=======================================
Files 90 90
Lines 16714 16717 +3
=======================================
+ Hits 15399 15402 +3
Misses 1315 1315
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds a uid to the
Nodebaseclass via theuuidmodule. Refer Issue #123Current implementation utilizes
uuid4()which creates ae.g.
UUID('d5181261-c06e-4f68-82ed-8f8d4592511b')Two implementation questions arise:
UUIDtype for loki's case a reasonable choice or would something simpler, i.e. a generator of incrementing integers, be enough?Commentclass was not calling theNodepost init function? As far as git blame goes there once was a call to the base class init_ but it was removed @mlange05 (d6fe4e1).Current tests are:
Nodetypes are collected with dummy constructor arguments and checked if behavior is as described above