astropy / pyregion

ds9 region parser for python
https://pyregion.readthedocs.io
MIT License
39 stars 38 forks source link

Release v2.0 #94

Closed cdeil closed 6 years ago

cdeil commented 7 years ago

FYI: I've added a v2.0 milestone and removed the v1.3 milestone.

My suggestion would be that we release v1.2 now, and reserve backward-incompatible changes to a v2.0 release, to be done in ~ 1 month.

There's many issues currently under the v2.0 milestone, but some of the key proposed changes that would justify a major version bump would be changes that are silently and sometimes slightly backward-incompatible like #89 or #35.

Another goal of 2.0 I think should be to get rid of the bundled Kapteyn like @sargas already attempted in #54 , so that pyregion can easily be packaged by Debian and others (they have to remove bundled files, it's not allowed).

@leejjoon , all - Sounds like a plan? Thoughts?

leejjoon commented 7 years ago

Sounds good to me.

cdeil commented 7 years ago

I've set the pyregion 2.0 milestone / release data to October 6. If someone wants to do the release (@sargas?) please comment here. Otherwise I can do it, it doesn't take much time.

The main change to use Astropy instead of Kapteyn was done by @sargas in #100.

Everyone - Please see https://github.com/astropy/pyregion/milestone/4 and help resolve those issues / review the PRs, or file new ones now if you think something should be changed for version 2.0.

IMO we should take this as our one chance to do all breaking changes (even if minor, like changing a function parameter name for consistency) and then (hopefully) keep the package stable again for the coming years.

We already have a few breaking changes: http://pyregion.readthedocs.io/en/latest/changelog.html#unreleased although they should be reflected more clearly in the changelog at least in this case: https://github.com/astropy/pyregion/pull/97#issuecomment-244715349 where results can change with the update without getting a warning / error.

astrofrog commented 7 years ago

Just a ping - it would be great to release 2.0 as pyregion 1.2 is broken with Numpy 1.11 due to some of the kapteyn code (which has been removed in master):

       """
>      lon = d2r( n.asarray(longlat[:,0],'d').flatten(1) )
E      SystemError: <built-in method flatten of numpy.ndarray object at 0x12b3b38f0> returned a result with an error set

/Users/tom/miniconda3/envs/dev/lib/python3.6/site-packages/pyregion/extern/kapteyn_celestial.py:117: SystemError
cdeil commented 7 years ago

@astrofrog - Agreed.

I'll do the the pyregions 2.0 release for next week Thursday (May 18): https://github.com/astropy/pyregion/milestone/4

astrofrog commented 6 years ago

@cdeil - just to check, do you still plan on doing the 2.0 release?

cdeil commented 6 years ago

There's still some open issues under the 2.0 milestone ( https://github.com/astropy/pyregion/milestone/4 ) but maybe I should just merge #113 and go ahead and make a release now? I'll ping on #65 once more asking if anyone has the expertise and time to have a closer look and resolve it soon.

cdeil commented 6 years ago

I've made the 2.0 release: https://pypi.org/project/pyregion/

Can someone please try it out?

@bsipocz - do you have time to do this update? https://github.com/conda-forge/pyregion-feedstock Or should I give it a try? Is it just a matter of editing meta.yaml and making a PR?

cdeil commented 6 years ago

Tag is also there: https://github.com/astropy/pyregion/releases/tag/2.0

cdeil commented 6 years ago

Docs build is also OK: http://pyregion.readthedocs.io/en/2.0/

bsipocz commented 6 years ago

@cdeil - Thanks for making the release! I can do the conda update if you like, normally it's super easy, only the version number and sha hash need to be updated.

cdeil commented 6 years ago

Draft for pyregion 2.0 release announcement email: https://gist.github.com/cdeil/c5f58f4398fb47403b49d5c6f60bf25b (I'll wait for the conda packages to become available, just wrote it now for review)

@astrofrog or anyone - Thoughts?

astrofrog commented 6 years ago

Looks good!

cdeil commented 6 years ago

I tried the conda package on Mac. There's this warning:

software/anaconda3/anaconda3/lib/python3.6/site-packages/pyregion/tests/test_get_mask.py::test_region
  /Users/deil/software/anaconda3/anaconda3/lib/python3.6/importlib/_bootstrap.py:205: RuntimeWarning: numpy.dtype size changed, may indicate binary incompatibility. Expected 96, got 88
    return f(*args, **kwds)

But all tests pass, so I'm assuming all is OK. If someone thinks this needs to be looked into, please open a new issue.

Release announced on astropy and astropy-dev mailling lists:

epaell commented 6 years ago

Just a note that there appears to be an incompatible change that causes aplpy (1.1.1) to fail when using the 2.0 version of pyregion. I've submitted an issue with aplpy (https://github.com/aplpy/aplpy/issues/369) but I haven't fully debugged the issue to work out why it occurs.

boada commented 6 years ago

I'm experiencing the same issue as @epaell. I gave some more details in https://github.com/aplpy/aplpy/issues/369.

cdeil commented 6 years ago

I'm closing this old issue; it was about discussing the v2.0 release, which went out last year and then I forgot to close here.

@epaell @boada - for v2.0 there was some rewrite to use Astropy internally, and it looks like that has broken something for WCS with more than two axes. I see #66 is still open and seems very much related, please either comment there, or file a new issue.