Skip to content

Conversation

@nmearl
Copy link
Contributor

@nmearl nmearl commented Mar 19, 2018

No description provided.

@bsipocz
Copy link
Member

bsipocz commented Mar 23, 2018

@nmearl - why the update to the helpers? The current master has the latest release, unless specutils already requires python 3.5+

@nmearl
Copy link
Contributor Author

nmearl commented Mar 24, 2018

@bsipocz Huh, I'm not sure. It wasn't intentional. I'll remove it.

@@ -1,3 +0,0 @@
[submodule "astropy_helpers"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure you need to keep this content around.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, seems to be what all the failing tests are complaining about. 😉 But trust me, I'm not trying to remove this stuff -- just seems like every once in awhile, the astropy_helpers submodule wants to fall apart for no particular reason. 😞

2. If the pull request adds functionality, the docs should be updated. Put
your new functionality into a function with a docstring, and add the
feature to the list in README.rst.
3. The pull request should work for Python 2.6, 2.7, 3.3, 3.4 and 3.5, and for PyPy. Check
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since there is no testing for python 2.6&2.7&3.3, this requirement also needs updating. Just to be on the safe side you can hard wire the minimum requirement in setup.py, so the package won't even try to install for older versions

@nmearl nmearl merged commit 85c688b into astropy:master Mar 28, 2018
SaOgaz pushed a commit that referenced this pull request Mar 25, 2019
Make proper glue dataset to hold moment maps
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants