Replies: 1 comment
-
@gweesip I do agree with having the argument types consistent throughout the package. If I remember correctly, the reduce method originally used to take a dictionary for the variable states but it was later changed to a list. I can not recall the reasoning behind it and haven't been able to figure out even after digging through some commit messages. But if everything works fine with having the argument as a dict, I would be fine with changing it back to dictionary. |
Beta Was this translation helpful? Give feedback.
0 replies
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
-
I noticed that working with different classes in the package can be cumbersome because they use different formats for what I would consider essentially the same thing. For example
Am I missing something here? I think it would make the package more user friendly if argument types were consistent across the entire package so you don't always have to refer to the docs to figure out the specific format of what you're trying to do. Type hinting (#1270) will likely help, but even with type hinting I think there is value in package consistency.
Unless this is intentional by design, I would consider elevating this as an issue, but not familiar enough with the code base to make that call. I realize that doing a package wide review would require considerable effort...
Beta Was this translation helpful? Give feedback.
All reactions