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

Python 2.7 #22

Merged
merged 16 commits into from
Jan 16, 2019
Merged

Python 2.7 #22

merged 16 commits into from
Jan 16, 2019

Conversation

amp180
Copy link
Contributor

@amp180 amp180 commented Jan 14, 2019

Made changes for python 2.7 compatibility, bumped minor version.

Resolves #21
Resolves #18

@amp180 amp180 mentioned this pull request Jan 14, 2019
Copy link
Owner

@erikwijmans erikwijmans left a comment

Choose a reason for hiding this comment

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

Overall looks good. Can you remove pointnet2/utils/__init__.pyc

tox.ini Show resolved Hide resolved
@erikwijmans
Copy link
Owner

Also, in the setup section of the README, can you remove the blurb about python3.6?

README.rst Outdated
* Install ``python3.6`` -- This repo is currently only tested with/officially supports ``python3.6``

All further instructions assume that `python` defaults to ``python3.6``
* Install ``python3.6``
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
* Install ``python3.6``
* Install ``python`` -- This repo is tested with ``2.7``, ``3.4``, ``3.5``, and ``3.6``

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't remember if 3.4 was a typo of 3.5 when I added it to tox.ini or whether I tested both. I'm going to run the setup and tests one more time on 3.4.9 and 3.5.6 just to be absolutely sure that it's good, but there's no reason it shouldn't be that I know of.

Copy link
Contributor Author

@amp180 amp180 Jan 16, 2019

Choose a reason for hiding this comment

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

3.4 was definitely a typo because torch 1.0.0 doesn't have a package for that version. I must have tested on 2.7 and then 3.6 twice.

$ pip3.4 install torch
Collecting torch
  Using cached https://files.pythonhosted.org/packages/5f/e9/bac4204fe9cb1a002ec6140b47f51affda1655379fe302a1caef421f9846/torch-0.1.2.post1.tar.gz
    Complete output from command python setup.py egg_info:
    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-build-hplx_sgk/torch/setup.py", line 11, in <module>
        raise RuntimeError(README)
    RuntimeError: PyTorch does not currently provide packages for PyPI (see status at https://github.com/pytorch/pytorch/issues/566).
    
    Please follow the instructions at http://pytorch.org/ to install with miniconda instead.

3.5 builds and passes tests now, 2.7 and 3.6 still pass and I've removed 3.4 from the readme and tox.ini.

…d the line that supplies a traceback from the exception re-raise in pytorch_utils.py, as python3.5 doesn't support supplying a traceback.
@erikwijmans
Copy link
Owner

This looks great! Thank you very much for your contributions :)

@erikwijmans erikwijmans merged commit 114c62e into erikwijmans:master Jan 16, 2019
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.

None yet

2 participants