Skip to content

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.

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...

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
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