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

Move gmt from requirements.txt to CI scripts instead #343

Merged
merged 3 commits into from
Oct 12, 2019

Conversation

weiji14
Copy link
Member

@weiji14 weiji14 commented Oct 11, 2019

Description of proposed changes

Edit: Simply placing gmt in our Continuous Integration (CI) scripts' CONDA_INSTALL_EXTRA environment variable so that the requirements.txt file remains valid for pip.

Renaming instances of requirements.txt -> conda-requirements.txt, and requirements-dev.txt -> conda-requirements-dev.txt.

The requirements.txt file is a standard file used by pip to install Python packages, but we use it here for installing conda packages?! There are actually some automated bots/services that gets confused by this requirements.txt file, thinking it should use if to install PyPI packages but fails because it's not meant for that.

May want to raise this upstream too at https://github.com/fatiando/continuous-integration.

Needed for Zeit Now continuous documentation PR at #344, may help resolve weiji14/deepbedmap#153

Reminders

  • Run make format and make check to make sure the code follows the style guide.
  • Add tests for new features or tests that would have caught the bug that you're fixing.
  • Add new public functions/methods/classes to doc/api/index.rst.
  • Write detailed docstrings for all functions/methods.
  • If adding new functionality, add an example to docstrings or tutorials.

Sorry, something went wrong.

The requirements.txt file is a standard file used by pip to install Python packages, and it is confusing to use it for installing conda packages.
@weiji14 weiji14 mentioned this pull request Oct 11, 2019
5 tasks
@weiji14 weiji14 requested a review from a team October 11, 2019 21:36
@seisman
Copy link
Member

seisman commented Oct 12, 2019

As far as I can see, all the packages in the requirements.txt and requirements-dev.txt can be installed via pip, except gmt=6.0.0rc4, right? As I said in #257, pygmt doesn't really dependent on the gmt package provided by conda-forge. I removed gmt from the requirements.txt files in #257, but it was added back in #311.

@weiji14
Copy link
Member Author

weiji14 commented Oct 12, 2019

Yes, the first problem is that the gmt=6.0.0rc4 line in requirements.txt doesn't work in the pip install case.

Second problem is that our CI build actually uses the requirements.txt file to install conda packages. Ideally we should be using an environment.yml (or similar) file to do that...

.travis.yml Outdated Show resolved Hide resolved
Move the conda gmt=6.0.0rc4 requirement from requirement.txt to the CONDA_INSTALL_EXTRA environment variable in our Travis and Azure build scripts.
@vercel
Copy link

vercel bot commented Oct 12, 2019

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/gmt/pygmt/3qtvtcmrl
🌍 Preview: https://pygmt-git-fork-weiji14-rename-conda-requirements.gmt.now.sh

@weiji14 weiji14 changed the title Rename requirements.txt to conda-requirements.txt Move gmt from requirements.txt to CI scripts instead Oct 12, 2019
@weiji14 weiji14 merged commit 91ff1c3 into GenericMappingTools:master Oct 12, 2019
@weiji14 weiji14 deleted the rename/conda-requirements branch October 12, 2019 01:35
@leouieda
Copy link
Member

This raises an interesting issue that we'll have to think about. If we publish to PyPI, even though the install would work, the user will get an error on import because GMT hasn't been installed as well. Which means that our PyPI packages are almost useless unless we bundle GMT and other binaries into a wheel (which I'm not looking forward to doing). It would be very frustrating for non-GMT users to pip install pygmt and not get something useful.

I don't have a solution for this. If anyone has some thoughts, that would be great!

weiji14 added a commit to weiji14/deepbedmap that referenced this pull request Oct 29, 2019
Includes GenericMappingTools/pygmt#343 that will hopefully address https://github.com/dependabot/feedback/issues/482. There's also the new `makecpt` wrapper added in GenericMappingTools/pygmt#329. Need to temporarily pin dask at 0.12.3 because of some multiprocessing error.

- [GMT](https://github.com/GenericMappingTools/gmt) from 6.0.0rc3 to 6.0.0rc5
  - [Release notes](https://github.com/GenericMappingTools/gmt/releases/tag/6.0.0rc5)
  - [Commits](GenericMappingTools/gmt@6.0.0rc3...6.0.0rc5)
- [PyGMT](https://github.com/GenericMappingTools/pygmt) from 0.0.1a0-43-g421e10d to 0.0.1a0-57-g7590d4a.
  - [Commits](weiji14/pygmt@0.0.1a0-43-g421e10d...0.0.1a0-57-g7590d4a)
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.

Dependabot needs permission to see pygmt
3 participants