-
-
Notifications
You must be signed in to change notification settings - Fork 3
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
MNT: Add noarch: python #13
MNT: Add noarch: python #13
Conversation
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( I do have some suggestions for making it better though... For recipe:
|
…da-forge-pinning 2020.11.18.20.19.01
Hi! This is the friendly automated conda-forge-linting service. I wanted to let you know that I linted all conda-recipes in your PR ( Here's what I've got... For recipe:
|
Hi! This is the friendly automated conda-forge-linting service. I just wanted to let you know that I linted all conda-recipes in your PR ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ping @moorepants (since you have more conda-forge experience), do you see an issue with moving to noarch Python builds here?
Also cc @seisman.
@@ -23,7 +24,7 @@ requirements: | |||
- numpy | |||
- packaging | |||
- pandas | |||
- python | |||
- python >=3.6 | |||
- xarray | |||
|
|||
test: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the pygmt package tested? I don't see any tests in the CI outputs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The imports are tested and that is sufficient here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is no import that would ensure the ctype code is executed, you can add an import that ensured that, or you can add a simple test file that ensures gmt function calls can be made.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could run pygmt.show_versions()
I suppose to check that things are loaded?
Edit: done, and it seems to work fine:
+ python -c 'import pygmt; pygmt.show_versions()'
PyGMT information:
version: v0.2.1
System information:
python: 3.9.0 | packaged by conda-forge | (default, Oct 14 2020, 22:59:50) [GCC 7.5.0]
executable: $PREFIX/bin/python
machine: Linux-4.15.0-1098-azure-x86_64-with-glibc2.12
Dependency information:
numpy: 1.19.4
pandas: 1.1.4
xarray: 0.16.1
netCDF4: 1.5.4
packaging: 20.4
ghostscript: 9.53.3
gmt: 6.1.1
GMT library information:
binary dir: $PREFIX/bin
cores: 2
grid layout: rows
library path: $PREFIX/lib/libgmt.so
padding: 2
plugin dir: $PREFIX/lib/gmt/plugins
share dir: $PREFIX/share/gmt
version: 6.1.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is that the test isn't run on windows anymore. I think you will need to check the installation on a Windows computer after this is merged to be sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I'll keep this PR on hold until tomorrow perhaps (when I can access a Windows computer at uni).
As long as pygmt is a pure Python package and there are no differences in the installation for different OSes it is fine. |
@@ -23,7 +24,7 @@ requirements: | |||
- numpy | |||
- packaging | |||
- pandas | |||
- python | |||
- python >=3.6 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also add to the python in host, I believe.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes, good catch.
How does pygmt interact with gmt? You aren't compiling a Python C extension or anything? |
That's a good question. I don't think we compile any C extensions, @seisman do you know anything about this? |
No C codes in the pygmt project. PyGMT calls GMT's shared library via ctypes. |
Co-authored-by: Dongdong Tian <[email protected]>
As long as that works universally on the 3 OSes, then you should be able to do to noarch: python. |
Ok, E pygmt.exceptions.GMTCLibError: Module 'psconvert' failed with status code 78:
E psconvert [ERROR]: System call [@gswin64c -q -dNOSAFER -dNOPAUSE -dBATCH -sDEVICE=bbox -DPSL_no_pagefill -dMaxBitmap=2147483647 -dUseFastColor=true "C:/Users/username/.gmt/sessions/gmt_session.3708/gmt_30.ps-" 2> "C:/Users/username/.gmt/sessions/gmt_session.3708/psconvert_1804c.bb"] returned error 1.
.conda\envs\pygmt\lib\site-packages\pygmt\clib\session.py:506: GMTCLibError But that's a separate issue I noticed before, and we'll sort that out upstream on GMT later. Anyways, thanks @moorepants for the review! |
Hi! This is the friendly automated conda-forge-webservice.
I've made the recipe
noarch: python
as instructed in #12.Here's a checklist to do before merging.