search

GenericMappingTools / pygmt

A Python interface for the Generic Mapping Tools.
https://www.pygmt.org
BSD 3-Clause "New" or "Revised" License
758 stars 220 forks source link

Release PyGMT 0.1.2 #501

Closed seisman closed 4 years ago

seisman commented 4 years ago

Release: v0.1.2 Scheduled Date: 2020/07/01

Before release:

Release:

After release:

3rd party update:

weiji14 commented 4 years ago

Sorry, got a bit caught up with a tricky PR in xarray and some other stuff. Anyhow, these are what I'd personally like to finish for the release:

Nice to haves:

There are still tests failing due to baseline image changes as mentioned in #451. I don't think #476 will necessarily resolve the test failures, but if we can get #462 up and running, we can at least make sure things work ok for GMT 6.1.0.

Also been wondering if we should just use pytest.mark.xfail to allow those grd* tests to fail in GMT 6.0, but not on GMT 6.1.0 (dependent on getting #462). That way all the tests will be green and we can release confidently.

weiji14 commented 4 years ago

Ok, I've changed my mind. Since GMT 6.1.0 is out :tada:, let's just make a PyGMT release soon-ish and deal with those PRs (e.g. #500, #476, #480, #462) in v0.2.0.

I've got a workaround PR in #503 to temporarily expect the 13 failures we're seeing due to baseline image changes. Once that's merged in, I'll finalize the changelog at #504 and we should be able to cut a release soon after.

MarkWieczorek commented 4 years ago

This is just to let you know that if you are planning a new release:

pygmt.grdimage is not working for me when using internal files with global projections (it used to work before, with the documented bugs). Everything, however, seems to work fine when passing netcdf filenames (which bypasses the pygmt virtual files). You can start by verifying the code in this issue:

https://github.com/GenericMappingTools/pygmt/issues/390

I have not debugged this too much, but I note that this is a problem for the projections Q and W, but not the hemispheric A.

pgmt: last commit on master gmt: 6.1.0

weiji14 commented 4 years ago

Hi @MarkWieczorek, thanks for the heads up. Can you clarify what you mean by 'not working'? I.e. is it a crash, or is there some other unexpected behaviour? Ideally we would test PyGMT v0.1.2 on both GMT 6.0 and 6.1, but I haven't got much time this week to do such extensive testing, but if it's quite critical, we could consider it.

Alternatively, it might be best to strongly recommend PyGMT v0.1.x users to stick with GMT 6.0, and reserve PyGMT v0.2.x for GMT 6.1 or above. The gridline/pixel registration and cartesian/geographic coordinate system issue we're trying to resolve in #476 is a very tricky beast, and will result in breaking changes for xarray users in the interim no matter how careful we are. Sticking with netcdf filenames would be a smart thing to do for now.

MarkWieczorek commented 4 years ago

Here is an example. The problem seems to be that pygmt can not project images when the central longitude is not 180 degrees. Changing the longitude to other values either gives incorrect output, or crashes.

import pygmt
# Creata a data array in gridline coordinates of sin(lon) * cos(lat)
interval = 10
lat = np.arange(90, -90-interval, -interval)
lon = np.arange(0, 360+interval, interval)
longrid, latgrid = np.meshgrid(lon, lat)
data = np.sin(np.deg2rad(longrid)) * np.cos(np.deg2rad(latgrid))

# create a DataArray
dataarray = xr.DataArray(data, coords=[('latitude', lat,
                                       {'units': 'degrees_north'}),
                                       ('longitude', lon,
                                       {'units': 'degrees_east'})], 
                         attrs = {'actual_range': [-1, 1]})
dataset = dataarray.to_dataset(name='dataarray')
dataset.to_netcdf('test.grd')

This works

# create projected images using a cylindrical projection.
fig = pygmt.Figure()
fig.grdimage(dataset.dataarray, region='g', projection='Q180/0/6i', frame='a90f30g30')
fig.show(method='external')

This crashses

# create projected images using a cylindrical projection.
fig = pygmt.Figure()
fig.grdimage(dataset.dataarray, region='g', projection='Q0/0/6i', frame='a90f30g30')
fig.show(method='external')

with this error

grdimage [ERROR]: Passing zmax <= zmin prevents automatic CPT generation!
grdimage [ERROR]: Failed to read CPT (null).
Error: /syntaxerror in /--%ztokenexec_continue--

...

GMTCLibError: Module 'psconvert' failed with status code 78:
psconvert [ERROR]: System call [gs -q -dNOSAFER -dNOPAUSE -dBATCH -sDEVICE=bbox -DPSL_no_pagefill -dMaxBitmap=2147483647 -dUseFastColor=true '/Users/lunokhod/.gmt/sessions/gmt_session.47287/gmt_17.ps-' 2> '/Users/lunokhod/.gmt/sessions/gmt_session.47287/psconvert_47287c.bb'] returned error 256.

Using a longitude of 270 completes, but provides incorrect output.

MarkWieczorek commented 4 years ago

A simple temporary fix to this problem would be to allow passing a netcdf object to pygmt, which would then open the netcdf object as a file (therefore bypassing the internal virtual file format:

Figure.grdimage(xarray_dataset.to_netcdf(), ...)

but unfortunately, this doesn't seem to work.

weiji14 commented 4 years ago

The to_netcdf() would just return None I think, you would need to pass the actual filename. And I suspect that GMT 6.1 probably breaks PyGMT v0.1.1 too?

MarkWieczorek commented 4 years ago

I'm using GMT6.1.0: I am trying to figure out how to reinstall 6.0.0, but brew on macOS won't let me do this (see this issue).

I have tried using pygmt==0.1.1 and 0.1.0 with GMT6.1.0, but this doesn't fix anything. Thus, it is possible that this is a GMT6.1 issue, but it is hard for me to believe that a minor version update of GMT would break something as fundamental as this.

seisman commented 4 years ago

it is possible that this is a GMT6.1 issue, but it is hard for me to believe that a minor version update of GMT would break something as fundamental as this.

The GMT C API is complicated and it supports many different ways to pass data from external wrappers to GMT API. As I understand it, PyGMT is using a different way than GMT.jl and gmtmex. That's why PyGMT discovers so many bugs in the GMT C API, while GMT.jl and gmtmex work well.

GMT 6.1 actually made some big changes to how data are passed to GMT, although the API functions are unchanged. That's why it's tricky to support both GMT 6.0 and 6.1.

As for the issue you reported, it's not clear if it's a GMT bug or a PyGMT bug. I'd like to release 0.1.2 ASAP so that we can bump the minimum GMT version to 6.1.0 and see what's wrong with the code.

weiji14 commented 4 years ago

To be fair, I don't think we're introducing any new regressions with PyGMT v0.1.2, the same problem exists on PyGMT v0.1.1 in regard to GMT 6.1.0. I'm really sorry that things are broken, but this has been a messy situation (there are probably a dozen PRs that tried to handle GMT 6.0/6.1 compatibility but got stuck). What we might do is this:

  1. Release PyGMT v0.1.1, and do a hard pin on GMT 6.0 on conda-forge so users don't install GMT 6.1 accidentally (or at least document it in the install instructions?).
  2. Bump GMT to 6.1 and fix all the bugs related to crashes, etc.
  3. Release v0.2.0 as soon as possible (this month)
weiji14 commented 4 years ago

@seisman, before I merge in #504, do you think we should update the install instructions at https://github.com/GenericMappingTools/pygmt/blob/master/doc/install.rst#installing-gmt-and-other-dependencies to say conda create --name pygmt gmt=6.0 instead of just gmt? Or would the hard pin on conda-forge be better?

seisman commented 4 years ago
  1. Release PyGMT v0.1.1, and do a hard pin on GMT 6.0 on conda-forge so users don't install GMT 6.1 accidentally (or at least document it in the install instructions?).

Perhaps not pin on GMT 6.0. With PyGMT v0.1.2 + GMT 6.1.0,

  1. all the existing tests pass (there are some failures, but we know they should pass)
  2. all the issues labeled with "upstream" should also pass. I think I tested all of them, and they all passed.
  3. the only issue we know for PyGMT v0.1.2 + GMT 6.1.0 is the one reported in #390. It fails for some projections but not all.

Considering the three points above, allowing the combination of PyGMT v0.1.2 + GMT 6.1.0 is a better choice.

weiji14 commented 4 years ago

Ok, so no hard pin. And I won't bother with the install instruction update then since things should work for most cases, we'll fix #390 and others in v0.2.0. Will merge in #504 now (and go have lunch).

seisman commented 4 years ago

Yes, good to me (and go have dinner).

weiji14 commented 4 years ago

Ok, I've pushed the v0.1.2 tag, and will make a release shortly. I'll also handle the Zenodo archive this time since I reserved the DOI, but I should let you have a go for v0.2.0.

weiji14 commented 4 years ago

Alright, conda-forge packages are up on https://anaconda.org/conda-forge/pygmt/files, and I've made the announcement on https://forum.generic-mapping-tools.org/t/pygmt-v0-1-2-released/629. Feel free to close this, and we can start to work on v0.2.0 :tada:

seisman commented 4 years ago

I also updated the project progress on ResearchGate (https://www.researchgate.net/project/PyGMT-A-Python-interface-for-the-Generic-Mapping-Tools).

seisman commented 4 years ago

Done with the v0.1.2 release. Closed.

weiji14 commented 4 years ago

Cool, thanks for that! I really need to get on that ResearchGate project, and we should add Liam as well. Do we need @leouieda to do it?

seisman commented 4 years ago

I think I can edit the collaborators and add your names, but it seems the names are not linked to your accounts.

weiji14 commented 4 years ago

Seems like we need to follow each other to be collaborators (see https://www.researchgate.net/post/how_can_I_add_collaborators_to_an_existing_project). Could you try that and see if it works?

seisman commented 4 years ago

It works. I added you but can't change the order of the names.

weiji14 commented 4 years ago

All good, I changed the order (just had to delete Paul's name first and add it back at the end).