Add conversion from sequence to immutable tuple #401
Conversation
| for element in sequence: | ||
| if not isinstance(element, first_element_type): | ||
| return "different type" | ||
| if any(not isinstance(element, first_element_type) for element in sequence): |
There was a problem hiding this comment.
How is this better than the previous implementation?
There was a problem hiding this comment.
Just a minor detail here. I think the original implementation may be slightly better because it stops at the very first element in sequence that is not an instance of first_element_type. In the proposed implementation here, the whole sequence is iterated through (I think it is, not completely sure), producing a list that then is passed to any.
There was a problem hiding this comment.
Makes sense. I have removed this implementation and left it as it was.
|
PR in response to issue #352 where it's suggested to convert client input sequence into tuple so that it's immutable (safe from odd manipulations of list attribute after setting). |
This suggested change is linked to #352 issue where we do not want attr value to be mutable lest the user accidentally modifies it.
Great suggestion! I have updated the test_attributes to assert equal to tuple rather than list. (I'm new to contributing to Open Source projects and Open Telemetry in general, so I'm still learning) |
|
@Mizkoeu if you sign the CLA this should be good to go. |
|
@ocelotl is there still a blocking issue for you? Looks like your concern was reverted. |
|
@Mizkoeu just a reminder that you'll need to sign the CLA before we can merge. Thanks! |
|
#449 replaced this. |
No description provided.