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

Don't use conda-env files with Travis-CI #1130

Merged
merged 1 commit into from
Jul 31, 2019
Merged

Conversation

ocefpaf
Copy link
Contributor

@ocefpaf ocefpaf commented Jul 31, 2019

I'll explain the details inline but the main change here is to avoid using conda-env environment files but use the same requirements files to instead to create the test env. This will prevent mismatch between the testing env and what people will get when installing pysal with wither pip or conda.

@@ -1,13 +1,18 @@
language: python
language: minimal
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If using conda for testing we don't need the image with Python.

Copy link
Member

Choose a reason for hiding this comment

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

Good to know.

- name: "python-3.7"
env: PY=3.7
- name: "tarball"
env: PY=3.7
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test matrix implements the two tests that existed, for 3.6 and 3.7, and add a new one to check the tarball created. This will help prevent some release issues.

@@ -18,17 +23,22 @@ install:
conda config --add channels conda-forge --force
conda config --set channel_priority strict
conda config --set safety_checks disabled
conda create --name TEST python=$PY --file requirements.txt --file requirements_dev.txt --file requirements_docs.txt --file requirements_plus.txt
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the "core" of this PR, it will create the environment for each python in the matrix based on the same files used in setup.py, so the conda test env and a pip installation are installing the same packages.


script:
- python setup.py sdist >/dev/null
- python setup.py install
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tarball test substitutes python setup.py sdist and pip install -e . --no-deps --force-reinstall is better then python setup.py install b/c it won't try to pull the dependencies and uses pip, as recommended by PyPA. (python setup.py install will be removed at some point.)

@@ -7,3 +7,5 @@ coveralls
bokeh
ipywidgets
statsmodels
twine
wheel
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Used in the tarball check.

@@ -1,6 +1,6 @@
sphinx>=1.4.3
sphinxcontrib-napoleon
sphinx_gallery
sphinx-gallery
Copy link
Contributor Author

Choose a reason for hiding this comment

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

pip does not care about - or _ but conda is unambiguous and accepts only one. This now works with both conda and pip.

@@ -7,6 +7,7 @@ geojson>=1.3.2
folium>=0.2.1
mplleaflet>=0.0.5
statsmodels>=0.6.1
mapclassify
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only dependency I ported from the env files. However, I'm not familiar with the pysal code and maybe the tests require more packages here. (Locally I noticed that 49 tests were skipped.)

@@ -60,6 +60,7 @@ def setup_package():
version=__version__,
description="A library of spatial analysis functions.",
long_description=long_description,
long_description_content_type="text/x-rst",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed a warning from the tarball check.

PS: I noticed that there are two README, a markdown and a ResT. PyPI now accepts Markdown, so if you want to join them into one I can update this PR or send a new one later.

Copy link
Member

Choose a reason for hiding this comment

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

That would be great!

@sjsrey
Copy link
Member

sjsrey commented Jul 31, 2019

Thanks for the detailed explanation. This is really helpful!

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.

2 participants