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 missing import of Pose3 #858

Merged
merged 1 commit into from
Aug 24, 2021
Merged

Add missing import of Pose3 #858

merged 1 commit into from
Aug 24, 2021

Conversation

varunagrawal
Copy link
Collaborator

Fixes bug accidentally introduced by #844.

@varunagrawal varunagrawal added the quick-review Quick and easy PR to review label Aug 23, 2021
@varunagrawal varunagrawal self-assigned this Aug 23, 2021
Copy link
Contributor

@johnwlambert johnwlambert 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 catching the missing import.

I am using python black https://github.com/psf/black. I find it to be the cleanest of all the Python auto-formatters. Could you take a look at it? If we could switch to it, I would be quite happy : - )

Python black is used by 73K repos. Yapf is used by just 22K repos, according to Github

@varunagrawal
Copy link
Collaborator Author

Yeah Github stars and users don't mean much since we follow the Google Style for Python (which recommends yapf).

@varunagrawal
Copy link
Collaborator Author

I also like black because it is very opinionated. Unfortunately, formatting all the python code is a chore I don't want to undertake any time soon.

@ProfFan
Copy link
Collaborator

ProfFan commented Aug 23, 2021

What is the biggest advantage of black over yapf?

@johnwlambert
Copy link
Contributor

What is the biggest advantage of black over yapf?

I think the function signatures are much easier to read with Black. For one example, YAPF formats this as:

def ellipsoid(rx: float, ry: float, rz: float,
              n: int) -> Tuple[np.ndarray, np.ndarray, np.ndarray]:

but Black formats it as:

def ellipsoid(
    rx: float, ry: float, rz: float, n: int
) -> Tuple[np.ndarray, np.ndarray, np.ndarray]:

I find the latter a lot easier to read and review.

Black is also quite fast: https://news.ycombinator.com/item?id=17155048

@varunagrawal varunagrawal merged commit dc148ed into develop Aug 24, 2021
@varunagrawal varunagrawal deleted the fix/missing-import branch August 24, 2021 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
quick-review Quick and easy PR to review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants