Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add type annotations #74

Open
Muream opened this issue May 28, 2022 · 5 comments
Open

Add type annotations #74

Muream opened this issue May 28, 2022 · 5 comments

Comments

@Muream
Copy link
Contributor

Muream commented May 28, 2022

Ive been using cmdx on and off for a few months now and I've been very happy with it!
My only (mild) pain point with it is the lack of type annotations

Would you be open for me to add that in?

I'm asking here before making a PR because I feel like this is something that should be supported by every contributor going forward so I don't want to start working on it if that's not something you want 🙂

Mypy can be used to ensure the annotations are good as part of the CI/CD so it can be enforced and made consistent.

Type annotations can either be using the python 3 syntax:

def createNode(type: Union[str, MTypeId], name: Optional[str] = None, parent: Optional[DagNode] = None) -> Node:
    ...

or the python 2 compatible comment based syntax:

def createNode(type, name=None, parent=None):
    # type: (Union[str, MTypeId], Optional[str], Optional[DagNode]) -> Node
    ...

The python 3 syntax is more comfortable to use though considering cmdx supports older versions of Maya, the comment based syntax is probably the only viable option. We use this at work and works very well with both Mypy and Pylance (Apparently PyCharm supports it as well)

Let me know your thoughts on this 👍

@mottosso
Copy link
Owner

Heya @Muream,

The python 3 syntax is more comfortable to use though considering cmdx supports older versions of Maya, the comment based syntax is probably the only viable option

Indeed, at least until Maya 2020 is deprecated, so for at least another 3 years.

Would you be open for me to add that in?

Yes, on 1 condition.

  • Line length

Sounds petty, but that has been my issue with these things in the past. I don't benefit from type hints personally, but if there's one thing I do benefit from it's multiple side-by-side editor views. From what I gather, the parsing mechanic behind defining these as comments is that the need to be on one line? What ends up happening then is that these lines grow beyond 80 chars wide, to 200+ depending on how many arguments there are. Even that small Python 2 example is dangerously close at 76 chars.

@Muream
Copy link
Contributor Author

Muream commented May 29, 2022

Haha yes, I always have at least two editors side by side so I hear you

It's perfectly possible to have type annotations per line in a multiline function definition like so

def createNode(
    type,  # type: Union[str, MTypeId]
    name=None,  # type: Optional[str]
    parent=None,   # type: Optional[DagNode]
):
    # type: (...) -> Node
    ...

Another option is to use a stub file. It's generally recommended to keep the annotations within the source file as that's just one file to maintain instead of two

But in this case, I think it might be the way to go since typing needs to be imported for more advanced type annotations (like generics) and that won't be available for python 2 either without having it as a dependency which I'm pretty sure you'd rather not have

@mottosso
Copy link
Owner

I like the idea of a stub file, in that it would have little (to none?) effect on those that don't use typing. But it does sound rather tedious on your part, having to write (or generate?) this file.

I'd be up for seeing the start of a PR with these comment-based type-hints, to see what kind of effect/damage it would have on readability. Maybe it's not that bad? If it is, then there's always the stub file.

@Muream
Copy link
Contributor Author

Muream commented May 29, 2022

Cool, generating a stub is easy with mypy and after that it's as much work cleaning it up as adding annotations to the source file itself and is actually likely to give me more freedom to implement things like Generics

I'll make a proof of concept PR in the next few days with a few different styles of annotations 👍

@mottosso
Copy link
Owner

Awesome!

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

No branches or pull requests

2 participants