Skip to content

Conversation

@yashsinghcodes
Copy link
Member

Handled it at the exception, as the did not wanted to interfere with set_key logic above it. Let me know if this looks good. I did test it and seems to me. That it should work and now can handle all the cases.

The only problem I do have is it really required to have the length of the two list to be same now? That we have a exception which handles it? Let me know.

@yashsinghcodes yashsinghcodes requested a review from frikky January 28, 2025 13:29
Copy link
Member

@frikky frikky left a comment

Choose a reason for hiding this comment

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

hmm

}
try:
# if nothing works lets try to add both list
list_one = list_one + list_two
Copy link
Member

Choose a reason for hiding this comment

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

"Merge" and "add list to list" are two entirely different things.

The description of the function is "Merges two lists of same type AND length."

Copy link
Member Author

Choose a reason for hiding this comment

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

But doesn't it mean we are creating constraints that can be handled easily? But I get it this was the design choice, I will update the health workflow to use add list to list when it comes to this.

Copy link
Member

Choose a reason for hiding this comment

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

But doesn't it mean we are creating constraints that can be handled easily? But I get it this was the design choice, I will update the health workflow to use add list to list when it comes to this.

Constraints are good and helps you understand what a function does. In this case, e.g. plus together each key instead of plusing the entire thing with ambiguous results

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