Skip to content
New issue

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

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

Already on GitHub? # 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
# for free to join this conversation on GitHub. Already have an account? # 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