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

Fix #1093 by moving requirements into environment.yml #1094

Merged
merged 1 commit into from
Dec 20, 2016

Conversation

zrisher
Copy link
Contributor

@zrisher zrisher commented Dec 8, 2016

To fix #1093:

  • I removed the contradicting pandas and numpy lines in requirements.txt.
  • I then found that numba == 0.26.0 also implicity declares numpy>=1.7. Numba was also in the environment.yml, but without a version constraint, so I moved this there.
  • six and toolz were declared in both places, so I removed them

That made requirements.txt empty, so I removed it. Happy to keep it in if preferred!

@codecov-io
Copy link

codecov-io commented Dec 8, 2016

Current coverage is 98.79% (diff: 100%)

Merging #1094 into master will not change coverage

@@             master      #1094   diff @@
==========================================
  Files            38         38          
  Lines          2830       2830          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           2796       2796          
  Misses           34         34          
  Partials          0          0          

Powered by Codecov. Last update ce88a73...cccca19

@zrisher zrisher changed the title move requirements into environment.yml Fix #1093 by moving requirements into environment.yml Dec 8, 2016
@talumbau
Copy link
Member

talumbau commented Dec 8, 2016

Ah, I see you did remove the requirements.txt. Ok, very good. Let's try it this way and see what breaks. I think the only possible issue will be what happens when I try to upload a new release to PyPI (https://pypi.python.org/pypi/taxcalc). If this breaks, I will find a way to fix it that does not involve creating a requirements.txt, since that eventually leads to dependencies specified in two places, which is bad.

@zrisher
Copy link
Contributor Author

zrisher commented Dec 9, 2016

I think the only possible issue will be what happens when I try to upload a new release to PyPI (https://pypi.python.org/pypi/taxcalc). If this breaks, I will find a way to fix it

I would really appreciate that. I have no idea how the environment is created when this is built as a package. I imagine it has something to do with this line in setup.py:

 'install_requires': ['numpy', 'pandas'],

The Travis environment doesn't use environment.yml either, but rather requirements entered directly via the command line:

 - conda create -n taxcalcdev python=$TRAVIS_PYTHON_VERSION pytest setuptools pandas=0.18 toolz six mock bokeh

It would be awesome if we could unify the environments for local users, package users, and testing at some point.

@zrisher
Copy link
Contributor Author

zrisher commented Dec 9, 2016

Just pushed a quick fix - the numba == 0.26.0 requirement was in that file from my local windows dev branch and accidently slipped into this PR. I removed it, so actually no changes to the environment.yml now, just the removal of requirements.txt.

@MattHJensen
Copy link
Contributor

@talumbau, could you merge this if it looks good to you?

@talumbau
Copy link
Member

looks good. merging.

@talumbau talumbau merged commit 3d24438 into PSLmodels:master Dec 20, 2016
@zrisher zrisher deleted the windows-build-fix-1 branch December 21, 2016 00:31
@zrisher zrisher mentioned this pull request Dec 22, 2016
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.

Conda dependency constraints overwritten by Pip requirements
4 participants