Skip to content

Conversation

@Relativiteit
Copy link

@Relativiteit Relativiteit commented Apr 23, 2023

add type hinting, document strings and typo in readme Resolves #13
Please test on linux machine before merging!!

@Relativiteit
Copy link
Author

@rdiankov @ziyan @liuhuanjim013 @kanunikov-denys if no longer needed please close :)

@cielavenir
Copy link

Currently we need to maintain Python2 as well. This pull request can be considered at least 1 year later.

@Relativiteit
Copy link
Author

@cielavenir alright, is there anything else more pressing currently you need help on ?

@cielavenir
Copy link

@felixvd @hemangandhi could you check?

@ziyan
Copy link
Member

ziyan commented Oct 16, 2024

@cielavenir Do you have a pipeline ID?

Copy link

@hemangandhi hemangandhi left a comment

Choose a reason for hiding this comment

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

Thanks for the type annotations!

@cielavenir
Copy link

@ziyan https://tiny.mujin.co.jp/qarwm

However, since this typing will drop py2 explicitly, @felixvd must approve explicitly.

Copy link
Collaborator

@felixvd felixvd left a comment

Choose a reason for hiding this comment

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

We have other Python3-style type annotation now, so LGTM!

@cielavenir
Copy link

ok to merge after minor syntax is corrected (this is year-old mr so @hemangandhi do you want to take a look at this? i pushed the same name branch to mujin)

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.

5 participants