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

Ensure install with Python3 #37

Closed
wants to merge 1 commit into from
Closed

Conversation

1papaya
Copy link

@1papaya 1papaya commented Jun 19, 2020

Force setup.py executable to use python3 and pip3

@codecov-commenter
Copy link

codecov-commenter commented Jun 19, 2020

Codecov Report

Merging #37 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #37      +/-   ##
==========================================
+ Coverage   85.98%   86.01%   +0.02%     
==========================================
  Files          18       19       +1     
  Lines         942      944       +2     
  Branches      175      175              
==========================================
+ Hits          810      812       +2     
  Misses         96       96              
  Partials       36       36              
Impacted Files Coverage Δ
pyrosm/__init__.py 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc62c92...9b28714. Read the comment docs.

@HTenkanen
Copy link
Collaborator

@geoDavey Thanks for the PR! Looks good, however, I think calling python3 or pip3 are Unix specific commands if I'm not mistaken? Hence, it might be that these changes break installation on other systems such as Windows. 🤔 Have you tried this on other OS than Linux?

from Cython.Build import cythonize

# Cykhash needs to be installed before running setup
try:
import cykhash
except ImportError:
os.system('pip install https://github.com/HTenkanen/cykhash/archive/master.zip')
os.system('pip3 install https://github.com/HTenkanen/cykhash/archive/master.zip')
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can actually be nowadays installed from PyPi directly.

@HTenkanen
Copy link
Collaborator

Closing this now. Pyrosm is available from conda-forge which is the recommended package manager for installing pyrosm (supports all operating systems and Python 3.6 onwards). So at this point in time, sorry, I have no plans to support pipenv due to some shortcomings of pipenv mentioned e.g. in here: https://towardsdatascience.com/guide-of-choosing-package-management-tool-for-data-science-project-809a093efd46 .

@HTenkanen HTenkanen closed this Jul 1, 2020
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

3 participants