Misc cleanup before 2.0 release #120

cdeil · 2017-10-13T11:50:05Z

I plan to do some misc fixes and cleanup here, before the 2.0 release.

Currently there are some fails in master:
https://travis-ci.org/astropy/pyregion/builds/287512820

If there's anything difficult or controversial, I'll ask. No need to review at this point.

Work in progress ...

cdeil · 2017-10-13T12:15:46Z

@bsipocz - Do you remember what the fix was to suppress this list intersphinx issue?
https://travis-ci.org/astropy/pyregion/jobs/287523737#L1007
I remember seeing it before, but can't remember or find it ...

coveralls · 2017-10-13T12:20:01Z

Coverage remained the same at 63.504% when pulling 60bfe13 on cdeil:cleanup into 7bde9dd on astropy:master.

coveralls · 2017-10-13T12:34:40Z

Coverage increased (+0.04%) to 63.542% when pulling f5bf208 on cdeil:cleanup into 7bde9dd on astropy:master.

…t relevant for most users

cdeil · 2017-10-13T12:42:11Z

Because of the list intersphinx issue, for now I've just allowed Sphinx warnings in the Python 2 docs build: 32fceb7
Very few people build the docs of this package anyways and there's really no reason why they should use Python 2 any more ...
If someone wants to change back, sure, just go ahead with a commit in master or a pull request.

bsipocz · 2017-10-13T12:43:29Z

@cdeil - probably the simplest is to add them as exceptions to a docs/nitpick-exceptions file.

bsipocz · 2017-10-13T12:45:50Z

btw, I'm doing a new helpers release right now, if you want to included that here (though probably nothing major is in there that would affect pyregions).

cdeil · 2017-10-13T12:49:40Z

I don't think there's a reason to do a helpers update for this package.

coveralls · 2017-10-13T12:56:08Z

Coverage increased (+0.04%) to 63.542% when pulling 32fceb7 on cdeil:cleanup into 7bde9dd on astropy:master.

cdeil · 2017-10-13T13:05:03Z

It looks like changes to PYTEST_HEADER_MODULES in conftest.py have no effect for this package? I'm always seeing the modules from Astropy core listed.
The conftest.py file is executed, but changes to PYTEST_HEADER_MODULES have no effect on the prinout!???
@bsipocz - Can you reproduce? Do you have an idea what's wrong?

bsipocz · 2017-10-13T13:09:21Z

I don't think there's a reason to do a helpers update for this package.

at all? That case I can remove it from the auto-update list.

bsipocz · 2017-10-13T13:10:53Z

I'll check on the conftest stuff soon.

cdeil · 2017-10-13T13:15:23Z

I would suggest to keep this package on the astropy-helpers update list. I'm just saying if there are no known issues with what we have and CI is all green and this package is already at a recent astropy-helpers v2, then it's OK to release here and have other astropy-helpers updates and other fixes in future releases, no?

bsipocz · 2017-10-13T13:24:38Z

@cdeil - PYTEST_HEADER_MODULES seems to be working for me, could you give more details of what you see?
Maybe are you on the astropy slack?

This is what I get:

================================================================================= test session starts ==================================================================================
platform darwin -- Python 3.5.4, pytest-3.2.2, py-1.4.34, pluggy-0.4.0

Running tests with pyregion version 2.0.dev320.
Running tests in lib.macosx-10.10-x86_64-3.5/pyregion docs.

Date: 2017-10-13T14:23:00

Platform: Darwin-14.5.0-x86_64-i386-64bit

Executable: /Users/bsipocz/.virtualenvs/astropy-dev/bin/python

Full Python Version: 
3.5.4 (default, Sep  8 2017, 18:45:31) 
[GCC 4.2.1 Compatible Apple LLVM 7.0.2 (clang-700.1.81)]

encodings: sys: utf-8, locale: UTF-8, filesystem: utf-8
byteorder: little
float info: dig: 15, mant_dig: 15

Numpy: 1.13.1
Matplotlib: 2.0.2
Pandas: 0.20.3
Astropy: 3.0.dev20330
Using Astropy options: remote_data: none.

rootdir: /private/var/folders/dc/hsm7tqpx2d57n7vb3k1l81xw0000gq/T/pyregion-test-g4owfq37, inifile: setup.cfg
plugins: arraydiff-0.2.dev0, xdist-1.20.0, mpl-0.8, mock-1.6.2, forked-0.2, cov-2.5.1
collected 45 items                                                                                                                                                                      

pyregion/tests/test_cube.py .
pyregion/tests/test_ds9_attr_parser.py ...
pyregion/tests/test_ds9_region_parser.py ....
pyregion/tests/test_get_mask.py .
pyregion/tests/test_parser_helper.py .
pyregion/tests/test_region.py ......................
pyregion/tests/test_region_numbers.py .......
pyregion/tests/test_wcs.py ......

cdeil · 2017-10-13T15:25:58Z

I put this in conftest.py:

from collections import OrderedDict
PYTEST_HEADER_MODULES = OrderedDict([
    ('numpy', 'numpy'),
    ('matplotlib', 'matplotlib'),
    ('Astropy', 'astropy'),
    ('spam', 'spam'),
])

and I still get this, which I think is what Astropy has:

Numpy: 1.13.3
Scipy: 0.19.1
Matplotlib: 2.1.0
h5py: 2.7.0
Pandas: 0.20.3

Why?

bsipocz · 2017-10-13T15:31:02Z

It inherits the default from astropy, not sure what happens if you try to override that variable, it seems that nothing, you can't override it.
But what I see on this branch is what we usually do, delete the keys that are unneeded, and add new ones. Is there a problem with that approach?

try:
    PYTEST_HEADER_MODULES['Astropy'] = 'astropy'
    del PYTEST_HEADER_MODULES['h5py']
    del PYTEST_HEADER_MODULES['Scipy']
except NameError:
    pass

cdeil · 2017-10-13T15:41:23Z

Is there a problem with that approach?

Yes, Astropy keeps getting new dependencies (like pandas now) that are not a dependency in affiliated packages. IMO it's simpler / cleaner to just have a separate / independent list here, instead of doing additions / deletions wrt Astropy core and having to change that over time.

Obviously this is not a big deal. If it's not clear how to do the list from scratch, I'll go back to the other scheme. @astrofrog - Maybe you have a tip?

bsipocz · 2017-10-13T15:48:07Z

OK, then it's really an issue for the package template. What I would do is to hack around with a list to keep and clear the dict automatically for everything else.
Happy to come up with something after I see the helpers release out of the door.

bsipocz · 2017-10-13T16:14:12Z

Hmm, actually, this can be fixed in astropy. The dict is defined separately in the plugin, and there is indeed no reason to keep pandas or h5py or any astropy specific entry there.

I would suggest to keep numpy and mpl there, but even the latter could be removed from the default one.

cdeil · 2017-10-14T15:03:05Z

@bsipocz - I followed your suggestion to clear the dict and then fill it how I like. Works fine. Thanks!

Thinking about it a bit, it's not really surprising that making a new PYTEST_HEADER_MODULES didn't work. One has to change the existing dict that is already defined elsewhere and only brought into this namespace via the * import at the top.

coveralls · 2017-10-14T15:14:48Z

Coverage increased (+0.04%) to 63.542% when pulling a01d89e on cdeil:cleanup into 7bde9dd on astropy:master.

bsipocz · 2017-10-14T15:29:25Z

pyregion/ds9_attr_parser.py

+ds9_shape_in_comment_defs = dict(
+    text=wcs_shape(CoordOdd, CoordEven),
+    vector=wcs_shape(CoordOdd, CoordEven,
+                     Distance, Angle),


no need for these line breaks any more

(ohh, never mind, haven't seen that this got merged in the meantime ;) )

Use external pytest

60bfe13

cdeil added bug enhancement labels Oct 13, 2017

cdeil added this to the 2.0 milestone Oct 13, 2017

cdeil self-assigned this Oct 13, 2017

Use with open to avoid keeping filehandles open indefinitely

cd29a47

cdeil mentioned this pull request Oct 13, 2017

ascircle region? #121

Closed

cdeil added 2 commits October 13, 2017 14:05

Add entry to .gitignore

3c2c74b

Fix example in get_mask

a06c84b

cdeil added 2 commits October 13, 2017 14:16

Remove astropy dev py2.7 build on travis-ci (no longer supported)

7fe7c8c

Add py 3.6 build and declare that it is supported in setup.py

f5bf208

cdeil added 4 commits October 13, 2017 14:35

Add description for 2.0 release in changelog

3509830

Remove slow parser info box on front page -- not very friendly and no…

4e5f49b

…t relevant for most users

Change conda install instructions to conda-forge channel

f604859

Allow warnings in Python 2 docs build

32fceb7

A little bit of code (PEP8) and docstring cleanup

665054e

bsipocz mentioned this pull request Oct 13, 2017

Relocate generic pytest plugins to separate repositories astropy/astropy#6606

Merged

Update PYTEST_HEADER_MODULES in conftest.py

a01d89e

cdeil merged commit 326a588 into astropy:master Oct 14, 2017

bsipocz reviewed Oct 14, 2017

View reviewed changes

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Misc cleanup before 2.0 release #120

Misc cleanup before 2.0 release #120

cdeil commented Oct 13, 2017

cdeil commented Oct 13, 2017

coveralls commented Oct 13, 2017

coveralls commented Oct 13, 2017

cdeil commented Oct 13, 2017

bsipocz commented Oct 13, 2017

bsipocz commented Oct 13, 2017

cdeil commented Oct 13, 2017

coveralls commented Oct 13, 2017

cdeil commented Oct 13, 2017

bsipocz commented Oct 13, 2017

bsipocz commented Oct 13, 2017

cdeil commented Oct 13, 2017

bsipocz commented Oct 13, 2017 •

edited

Loading

cdeil commented Oct 13, 2017

bsipocz commented Oct 13, 2017 •

edited

Loading

cdeil commented Oct 13, 2017

bsipocz commented Oct 13, 2017

bsipocz commented Oct 13, 2017

cdeil commented Oct 14, 2017

coveralls commented Oct 14, 2017

bsipocz Oct 14, 2017

bsipocz Oct 14, 2017

Misc cleanup before 2.0 release #120

Misc cleanup before 2.0 release #120

Conversation

cdeil commented Oct 13, 2017

cdeil commented Oct 13, 2017

coveralls commented Oct 13, 2017

coveralls commented Oct 13, 2017

cdeil commented Oct 13, 2017

bsipocz commented Oct 13, 2017

bsipocz commented Oct 13, 2017

cdeil commented Oct 13, 2017

coveralls commented Oct 13, 2017

cdeil commented Oct 13, 2017

bsipocz commented Oct 13, 2017

bsipocz commented Oct 13, 2017

cdeil commented Oct 13, 2017

bsipocz commented Oct 13, 2017 • edited Loading

cdeil commented Oct 13, 2017

bsipocz commented Oct 13, 2017 • edited Loading

cdeil commented Oct 13, 2017

bsipocz commented Oct 13, 2017

bsipocz commented Oct 13, 2017

cdeil commented Oct 14, 2017

coveralls commented Oct 14, 2017

bsipocz Oct 14, 2017

Choose a reason for hiding this comment

bsipocz Oct 14, 2017

Choose a reason for hiding this comment

bsipocz commented Oct 13, 2017 •

edited

Loading

bsipocz commented Oct 13, 2017 •

edited

Loading