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

Translate transform for pointclouds #255

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

pushkalkatara
Copy link
Member

This PR solves issue #250

@pushkalkatara
Copy link
Member Author

pushkalkatara commented May 9, 2020

The build is failing at weird error -

/opt/conda/lib/python3.6/site-packages/numpy/core/include/numpy/npy_1_7_deprecated_api.h:17:2: warning: #warning "Using deprecated NumPy API, disable it with " "#define NPY_NO_DEPRECATED_API NPY_1_7_API_VERSION" [-Wcpp]
--
831 | #warning "Using deprecated NumPy API, disable it with " \
832 | ^
833 | imagecodecs/_aec.c:602:20: fatal error: libaec.h: No such file or directory
834 | compilation terminated.
835 | error: Setup script exited with error: command 'gcc' failed with exit status 1

maybe adding apt-get install libaec-dev would solve build error

@pushkalkatara
Copy link
Member Author

Hi @krrish94 , @Caenorst This transform is a preprocessing function for semantic segmentation in DGCNN pipeline. Please review the PR.

@Caenorst
Copy link
Collaborator

Hi @pushkalkatara , I think the error that you faced might be due to recent numpy release, we fixed it in #262 can you rebase and force push ?

@pushkalkatara
Copy link
Member Author

Thanks for pointing out @Caenorst Now the build is passing :D

Copy link
Collaborator

@Caenorst Caenorst left a comment

Choose a reason for hiding this comment

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

Thank you for your interest and contribution for Kaolin !

There are some comments that will need to be addressed, otherwise it's looking good ! :)

kaolin/transforms/pointcloudfunc.py Outdated Show resolved Hide resolved
kaolin/transforms/pointcloudfunc.py Outdated Show resolved Hide resolved
kaolin/transforms/pointcloudfunc.py Outdated Show resolved Hide resolved
kaolin/transforms/pointcloudfunc.py Outdated Show resolved Hide resolved
kaolin/transforms/pointcloudfunc.py Outdated Show resolved Hide resolved
kaolin/transforms/pointcloudfunc.py Outdated Show resolved Hide resolved
kaolin/transforms/transforms.py Outdated Show resolved Hide resolved
kaolin/transforms/pointcloudfunc.py Outdated Show resolved Hide resolved
kaolin/transforms/pointcloudfunc.py Show resolved Hide resolved
tests/transforms/test_transforms.py Show resolved Hide resolved
Signed-off-by: pushkalkatara <[email protected]>
@pushkalkatara
Copy link
Member Author

Hi @Caenorst Thanks for the detailed review. I have made some changes in the recent commit and added questions as well. please review once again. Thanks.

Copy link
Collaborator

@Caenorst Caenorst left a comment

Choose a reason for hiding this comment

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

a little comment about examples

kaolin/transforms/pointcloudfunc.py Outdated Show resolved Hide resolved
kaolin/transforms/pointcloudfunc.py Show resolved Hide resolved
Signed-off-by: pushkalkatara <[email protected]>
@Caenorst
Copy link
Collaborator

Caenorst commented Jun 1, 2020

Sorry, forgot to confirm the review :/

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.

3 participants