energy volume curve - use axes = ("x", "y", "z") rather than axes = ["x", "y", "z"] #63
Conversation
There was a problem hiding this comment.
Nice. The only thing is see is that the type hint probably needs to be more flexible, ie str str str, str str, or str. They could probably also be more specific with literals instead of str, but then the type hints for 2 and 1 cases each have multiple cases. At that point I'd probably define a custom type with all those cases, like axes_hint = typing.Union(tuple[Literal["x"{, ...), ...) or whatever, and then just hint the custom type. On the other hand that may be absolute overkill. I'm already content as-is, but just wanted to float the typing idea for your consideration.
|
@liamhuber I have to admit I am not really sure how the final type hint should look like. So if you want to make a suggestion or a stacked pull request I am happy to wait with merging this one, otherwise I would go ahead and merge this one as is. |
|
The hints are non binding in python anyhow; this one is by no means wrong, perhaps just narrower than the reality of the options; and I won't be able to get to it super fast -- so I'm fine with merging as is. Would you do me a favour and raise an issue linked to this and assign me to it though? Then I can get back to it. |
No description provided.