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

Release PyGMT 0.1.2 #501

Closed
9 of 10 tasks
seisman opened this issue Jul 1, 2020 · 23 comments
Closed
9 of 10 tasks

Release PyGMT 0.1.2 #501

seisman opened this issue Jul 1, 2020 · 23 comments
Labels
maintenance Boring but important stuff for the core devs
Milestone

Comments

@seisman
Copy link
Member

seisman commented Jul 1, 2020

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

Before release:

Release:

  • Make a tag and push it to Github
git tag vX.Y.Z
git push --tags

After release:

  • Create branch 0.x for bug-fixes if this is a minor release (i.e. create branch 0.1 after 0.1.0 is released)
  • Commit changes to Github

3rd party update:


  • Party 🎉 (don't tick before all other checkboxes are ticked!)
@seisman seisman added the maintenance Boring but important stuff for the core devs label Jul 1, 2020
@seisman seisman added this to the 0.1.x milestone Jul 1, 2020
@seisman seisman added this to To do in Release v0.1.x via automation Jul 1, 2020
@seisman seisman pinned this issue Jul 1, 2020
@weiji14
Copy link
Member

weiji14 commented Jul 2, 2020

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
Copy link
Member

weiji14 commented Jul 5, 2020

Ok, I've changed my mind. Since GMT 6.1.0 is out 🎉, 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.

@weiji14 weiji14 moved this from To do to In progress in Release v0.1.x Jul 5, 2020
@MarkWieczorek
Copy link
Contributor

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:

#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
Copy link
Member

weiji14 commented Jul 5, 2020

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

weiji14 commented Jul 6, 2020

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
Copy link
Contributor

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
Copy link
Member Author

seisman commented Jul 6, 2020

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
Copy link
Member

weiji14 commented Jul 7, 2020

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
Copy link
Member

weiji14 commented Jul 7, 2020

@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
Copy link
Member Author

seisman commented Jul 7, 2020

  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 Cylindrical projections 'Q' plot incorrectly in pygmt when using xarrays #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
Copy link
Member

weiji14 commented Jul 7, 2020

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
Copy link
Member Author

seisman commented Jul 7, 2020

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

@weiji14
Copy link
Member

weiji14 commented Jul 7, 2020

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
Copy link
Member

weiji14 commented Jul 7, 2020

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 🎉

@seisman
Copy link
Member Author

seisman commented Jul 7, 2020

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

@seisman
Copy link
Member Author

seisman commented Jul 7, 2020

Done with the v0.1.2 release. Closed.

@seisman seisman closed this as completed Jul 7, 2020
Release v0.1.x automation moved this from In progress to Done Jul 7, 2020
@seisman seisman unpinned this issue Jul 7, 2020
@weiji14
Copy link
Member

weiji14 commented Jul 7, 2020

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
Copy link
Member Author

seisman commented Jul 7, 2020

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

@weiji14
Copy link
Member

weiji14 commented Jul 7, 2020

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
Copy link
Member Author

seisman commented Jul 7, 2020

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

@weiji14
Copy link
Member

weiji14 commented Jul 7, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs
Projects
No open projects
Development

No branches or pull requests

3 participants