Moved astropy.vo.samp to astropy.samp #6213

astrofrog · 2017-06-15T15:07:32Z

@taldcroft and @mhvk agreed to this plan in #6201

@eteq @kelle @pllim - any objections to this?

@pllim - I actually ended up just removing docs/vo and adding the note you had added about astroquery to the what's new instead. Does that seem ok?

pllim · 2017-06-15T15:12:03Z

Wait a minute... Are you removing astropy.vo without deprecation? I don't plan to remove Cone Search until v3; I just deprecated it for v2...

bsipocz · 2017-06-15T15:13:00Z

@pllim - But conesearch got moved out, not just shuffled around.

pllim

I think completely removing vo without deprecation period is too drastic. I propose we deprecate vo.samp first in v2; i.e., anyone calling vo.samp will get a deprecation warning and vo.samp is actually a wrapper around samp. Then we can remove the whole vo sub-package in v3. What do you think?

pllim · 2017-06-15T15:13:12Z

astropy/astropy.cfg

 ## Conesearch database name.
 # conesearch_dbname = conesearch_good

 [vo.samp]


Don't you need a new [samp] section too?

astrofrog · 2017-06-15T15:14:49Z

@pllim - no I've only removed docs/vo not astropy.vo - that is, the imports will continue to work but the docs will go away. But I'm happy to add back the docs if you'd prefer to remove that section after 2.0.

pllim · 2017-06-15T15:16:48Z

@astrofrog , moving doc only is probably okay, but the diff also shows you moved the codes and deleted some cone search stuff.

astrofrog · 2017-06-15T15:18:21Z

@pllim - I just moved the files up one directory, and astropy/vo/samp/__init__.py now imports from astropy.samp so it's backward compatible. I didn't touch any of the cone search stuff as far as I know?

In [2]: from astropy.vo.samp import SAMPClient
WARNING: AstropyDeprecationWarning: The astropy.vo.samp module has now been moved to astropy.samp [astropy.vo.samp]

astropy.vo.client and astropy.vo.validator are still there!

pllim · 2017-06-15T15:27:33Z

@astrofrog , okay, I was just freaking out of all the cone search doc deletion. 😅

It just seems confusing to a user that they can still use astropy.vo but the documentation is gone. Maybe retain a vo/index.rst with the text you put in What's New? Not all users will dig into What's New.

EDIT: Would be nice to also retain the "Other third-party Python packages..." text until we completely get rid of astropy.vo.

Also need to update the "stability" listing.

astrofrog · 2017-06-15T17:50:43Z

@pllim - I've undeleted the vo docs!

pllim · 2017-06-15T18:19:07Z

Thanks! Still need a change log and edits to docs/stability.rst.

astrofrog · 2017-06-15T18:21:19Z

@pllim - done!

astrofrog · 2017-06-15T18:21:51Z

Before we merge this, just a reminder that I'd like to make sure @eteq is happy with this

pllim · 2017-06-15T18:25:31Z

CHANGES.rst


  - The `astropy.vo.samp` package no longer supports SSL. [#6201]

+  - The `astropy.vo.samp` package has been moved to `astropy.samp`. [#6213]


Nitpick: This entry and the one for #6201 above are a bit confusing. Maybe merge them into a single entry and reword in a more coherent manner?

pllim · 2017-06-15T18:26:43Z

docs/stability.rst

        <tr>
            <td>
-                astropy.vo.samp
+                astropy.samp


Maybe it is clearer to state in the description that it was renamed from astropy.vo.samp in v2.0? Other entries have info like that (when added, etc).

bsipocz · 2017-06-15T20:23:51Z

Added @eteq as a reviewer, as he needs to be happy with this move :)

eteq

One very minor comment, which could be changed at merge since it's just a doc change. Once the tests are done I think this is all set.

eteq · 2017-06-15T23:05:16Z

docs/vo/index.rst

+    The ``astropy.vo`` module has been deprecated and will be removed
+    in a future version. The SAMP functionality has been moved
+    to ``astropy.samp``, and the conesearch functionality has been
+    moved to `Astroquery <http://astroquery.readthedocs.io>`_.


Shouldn't this also mention that this is the "classic" API?

@eteq , are you talking about Cone Search or SAMP's "classic" API?

Oops! Sorry, misunderstanding: I thought the what's new page was saying that there "classic" API is in astropy, but I get now that that's referencing the astroquery version. So no change needed.

bsipocz · 2017-06-16T01:01:21Z

CHANGES.rst

    [#5558, #5904]

-  - The `astropy.vo.samp` package no longer supports SSL. [#6201]
+  - The `astropy.vo.samp` package has been moved to `astropy.samp`, and no


use double backticks to make sphinx happy

[ci skip]

eteq · 2017-06-16T01:40:06Z

Looks like the tests passed except for that one backtick issue that I fixed directly in this branch. Merging!

astrofrog force-pushed the move-samp branch from e841dfd to 69c63bd Compare June 15, 2017 15:07

pllim reviewed Jun 15, 2017

View reviewed changes

astrofrog added 7 commits June 15, 2017 18:34

Moved astropy.vo.samp to astropy.samp

0957b3a

Updated imports and renamed astropy.vo.samp to astropy.samp

814cede

Update instances of astropy.vo.samp in docs

a5002d9

More updates following move from astropy.vo.samp to astropy.samp

ba343cc

Added backward-compatibility layer for astropy.vo.samp

e64a9f2

Fix imports

a35038a

Update section title in astropy.cfg

9e85bc7

pllim added Refactoring vo samp labels Jun 15, 2017

pllim added this to the v2.0.0 milestone Jun 15, 2017

Add back VO docs to index but put a deprecation warning

ffe8500

astrofrog force-pushed the move-samp branch from 678789e to ffe8500 Compare June 15, 2017 17:49

pllim reviewed Jun 15, 2017

View reviewed changes

astrofrog force-pushed the move-samp branch from a5a7121 to 8448bcb Compare June 15, 2017 18:29

Updated stability docs and added changelog entry

a17aefb

astrofrog force-pushed the move-samp branch from 8448bcb to a17aefb Compare June 15, 2017 18:30

bsipocz requested a review from eteq June 15, 2017 20:23

eteq approved these changes Jun 15, 2017

View reviewed changes

bsipocz added the zzz 💤 merge-when-ci-passes Do not use: We have auto-merge option now. label Jun 15, 2017

bsipocz reviewed Jun 16, 2017

View reviewed changes

Add backticks to make docs build correctly

f7f8168

[ci skip]

eteq merged commit 7b2c9d2 into astropy:master Jun 16, 2017

pllim mentioned this pull request Jun 16, 2017

Remove astropy.vo sub-package #5558

Closed

pllim mentioned this pull request Jun 30, 2017

Ensure SAMP and FITS compatibility with Astropy 2.0 ejeschke/ginga#520

Merged

MSeifert04 removed the zzz 💤 merge-when-ci-passes Do not use: We have auto-merge option now. label Oct 17, 2017

astrofrog deleted the move-samp branch November 14, 2018 15:28


		- The `astropy.vo.samp` package no longer supports SSL. [#6201]

		- The `astropy.vo.samp` package has been moved to `astropy.samp`. [#6213]

Uh oh!

Moved astropy.vo.samp to astropy.samp #6213

Moved astropy.vo.samp to astropy.samp #6213

Uh oh!

Conversation

astrofrog commented Jun 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pllim commented Jun 15, 2017

Uh oh!

bsipocz commented Jun 15, 2017

Uh oh!

pllim left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astrofrog commented Jun 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pllim commented Jun 15, 2017

Uh oh!

astrofrog commented Jun 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pllim commented Jun 15, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

astrofrog commented Jun 15, 2017

Uh oh!

pllim commented Jun 15, 2017

Uh oh!

astrofrog commented Jun 15, 2017

Uh oh!

astrofrog commented Jun 15, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bsipocz commented Jun 15, 2017

Uh oh!

eteq left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eteq commented Jun 16, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

astrofrog commented Jun 15, 2017 •

edited

Loading

astrofrog commented Jun 15, 2017 •

edited

Loading

astrofrog commented Jun 15, 2017 •

edited

Loading

pllim commented Jun 15, 2017 •

edited

Loading