Thanks! While I think of it, it might make sense to think about is to split up the main core.py file into multiple smaller ones for readability.

Another quick (minor) note - I think the script should be renamed to samp_hub

I though to split the code in 3 modules:

• Hub stuff
• Client stuff
• Utilities (like the pure XMLRPC stuff)

I would avoid to change the script name for two reasons:

1. people are already used to launch sampy and probably they already have services and codes that rely on this name (I have, for example)
2. samp_hub is too generic, while sampy is more "characteristic" - you know it comes from Astropy. The other Hub implementation is JSAMP, and it preserves its characteristics in the name...

@lpaioro - I agree with the structure for the splitting. For the script name, I don't feel strongly about it, so we can leave it as such :)

Here's a small pull request: lpaioro#2

@lpaioro and @astrofrog At least for me samp isn't working yet, so before we continue with cleanup / refactoring,
can you look into the following issues?

• sampy command line tool doesn't work (see error here).
• python setup.py test --package vo unit test doesn't work (see error here).
• Eclipse PyDev static code analysis gives these four errors (could be real or false positives ... should add unit tests):

Description Resource    Path    Location    Type
Duplicated signature: _notifyAll    core.py /astropy/astropy/vo/samp    line 2537   PyDev Problem
Undefined variable: _hubAsClientRequestHandler  core.py /astropy/astropy/vo/samp    line 1732   PyDev Problem
Undefined variable: keyfile core.py /astropy/astropy/vo/samp    line 3254   PyDev Problem
Undefined variable: certfile    core.py /astropy/astropy/vo/samp    line 3254   PyDev Problem

@cdeil - FWIW, the test passes for me with Anaconda.

@cdeil - small detail - I noticed you added the BSD license line at the top, but the GPL license still needs to be removed from core.py.

Probably best if @lpaioro does it, I shouldn't have touched the license because I didn't trace back who contributed to that code and ask their permission.

I am going to remove the license. By the way, I had some automatic keywords added to the file after an svn commit. Is there something similar in git?

@lpaioro - it's not really common to do that in Git, but if you really need it, then Git supports post-commit hooks but I haven't used them before. Is there a reason why you need to continue updating these? GitHub makes it clear when the commit was made, and who made it, so maybe it's not necessary?

No, as a matter of fact it is not necessary. I think to remove them.

Currently there are these errors with Python 3 on travis-ci:

https://travis-ci.org/astropy/astropy/jobs/6385306#L652 :

Can't parse docstring in build/lib.linux-x86_64-3.2/astropy/vo/samp/core.py line 148: ParseError: bad input: type=11, value=':', context=('', (148, 12))
Can't parse docstring in build/lib.linux-x86_64-3.2/astropy/vo/samp/core.py line 158: ParseError: bad input: type=11, value=':', context=('', (158, 4))
Can't parse docstring in build/lib.linux-x86_64-3.2/astropy/vo/samp/core.py line 172: ParseError: bad input: type=11, value=':', context=('', (172, 4))
Can't parse docstring in build/lib.linux-x86_64-3.2/astropy/vo/samp/core.py line 4004: ParseError: bad token: type=55, value=' ', context=('', (4004, 71))
Can't parse docstring in build/lib.linux-x86_64-3.2/astropy/vo/samp/core.py line 4005: ParseError: bad input: type=5, value='            ', context=

https://travis-ci.org/astropy/astropy/jobs/6385306#L1035 :

astropy/vo/samp/core.py:1090: in <module>
>     class HTTPS(http.client.HTTP):
E       AttributeError: 'module' object has no attribute 'HTTP'

And locally I still get this error I mentioned before (full log here):

Exception AttributeError: "'SAMPHubServer' object has no attribute '_is_running'" in <bound method SAMPHubServer.__del__ of <astropy.vo.samp.core.SAMPHubServer obj

@lpaioro @astrofrog Any idea how to resolve these issues?

I think that this issue:

https://travis-ci.org/astropy/astropy/jobs/6385306#L652

is not really important, since it refers to some docstring parsing errors which will probably disappear once the documentation is rewritten in the proper Astropy compliant format.

This issue:

https://travis-ci.org/astropy/astropy/jobs/6385306#L1035

looks to be more complicated. When started I writing SAMPy, Python 3 was at the very beginning and the only way to provide secure http services was extending the normal HTTP classes provided by Python 2.6. Therefore I had to write HTTPSConnection class and the related classes. Now I see that Python 3 provides a native implementation of HTTPSConnection, so probably that part should be completely rewritten handling the two cases: Python 2.x and Python 3.x.

@keflavich is the main astroquery dev and knows a lot about HTTP in Python. @keflavich, maybe you can give advice or help re-writing sampy to be Python 3 compatible?

My experience with python 3 is pretty slim, but it may may sense to switch to using the requests module. I'll try to find time to look tomorrow.

@keflavich requests would probably be very nice to use here and possibly in any other web-related code. If someone is considering bundling requests in astropy/extern, here's some basic info:

• pure-Python, simultaneous Python 2 / 3 codebase it's quite a lot of files and 300 kB tarball on PyPI.
• grep import *.py */*.py */*/*.py | grep url shows that web-related code is used at the moment in the following modules: coordinates/name_resolve.py, utils/data.py, utils/misc.py, io/fits/file.py, io/votable/tree.py, utils/xml/check.py ... and soon vo/samp.

I always consider adding another dependency cautiously, but requests is awesome, and is widely considered the recommended way to do these things these days. I think it would simplify a number of things throughout astropy -- particularly in the forthcoming VO query stuff.

I've made a separate issue #1035 to discuss whether we should use requests or not.

Whilst at a glance requests looks to be very nice, I think that it doesn't give enough answers to the problems related with SAMP and its porting to Python 3.x. This because requests provides a very nice API client side for the normal HTTP request, but SAMP is more about XMLRPC, both client and server side.

Finally I modified the code in order to support Python 3 as well, but for the moment I don't use requests. I have added some tests also, but the testing code is a little rough.

@lpaioro When I have TOPCAT running the astropy.vo.sampy unit tests fail like so:
https://gist.github.com/cdeil/6173803

Is it possible to make them succeed even if another SAMP hub is already running on the user machine?

I had a look at the travis-ci error and I'm pretty sure some of them are random / old issues.
So @lpaioro, could you please rebase on trunk so that we see if there are real issues?

It would be nice if this PR makes it into the 0.3 release, which is set for ~ mid September.

From the tasks listed above I can do the polishing (code style, docs) in a PR against this one ... but first all tests should pass so that I know I'm not screwing up anything ...

OK @cdeil , if my personal conflict with git it's all water under the bridge, I rebased on trunk. I would be really happy to make astropy.vo.sampy into the 0.3 release.

travis-ci is having a bad day (timeouts when trying to install dependencies).

But this looks like a real failure with TestSAMPCommunication.test_2_clients:
https://travis-ci.org/astropy/astropy/jobs/9946823#L1202

@lpaioro Do you understand what the problem is from the test output?

It looks like the hub is not able to open a socket on the Travis host: https://travis-ci.org/astropy/astropy/jobs/9977929#L2903 . Do I have to specify the @remote_data annotation to make it working?

@lpaioro - I think it would make sense indeed to disable this test by adding the @remote_data decorator. I think those tests will still run on the Jenkins instances, just not the Travis instances.

@cdeil @lpaioro - congrats on the first commit passing all the Travis tests! At this rate we might be able to include this in 0.3! Keep up the good work :)

What remains to be done at this stage?

@astrofrog - I made a command decision and pushed this to 0.4, as I suspect this won't be done in a few days. IF I'm proven wrong it can be switched back.

@eteq - sensible move, I don't think this can be finished and fully tested in just 2-3 days.

@cdeil @lpaioro - this will need to be rebased, due to the reorganization that occured in docs/vo in preparation for this pull request.

It would be nice to get this done soon so that it can benefit from a few months of testing before the next major release.

@cdeil @lpaioro - I think it would make sense to try and get this ready as soon as possible after the 0.3 release so it can benefit from maximum testing before the next release. What is the status on this? What are the main difficulties in getting it done?

@astrofrog I don't think there's any major difficulties other than the fact that @lpaioro and I didn't have time to finish this up. I completely agree this should be ready soon and get into master to get some testing / usage. I don't have time right now, but I've put this in my calendar for next weekend.

@cdeil - thanks for the update! Remember that it's also possible to ask for help on astropy-dev if you need help. @lpaioro and you are managing this, but if it's a task that requires extra helpers, there are a few people looking for things to help with!

@lpaioro - could you rebase this? It looks like there is a conflict due to the re-arranging of the docs for astropy.vo in preparation for the present PR.

Sorry for being so slow answering, but actually I'm no more working for the astronomical community, I've been hired by a private industry and therefore I have very little time to dedicate to this project. However I'll try to do my best to follow the evolution of Astropy and contribute to it as a personal initiative to an open source project. I'm going to rebase astropy.vo.

@lpaioro - thanks for the update! Since your time is limited, once you have rebased your branch, I think the best approach may be for @cdeil (or whoever can lead this) to open a new PR from your branch, which will allow them to move forward with this (and we can then close this issue).

Yesterday night I rebased astropy.vo.samp, git told me that everything was OK, but I cannot see any change. Do you?

It still looks like it needs rebasing here - are you rebasing onto the latest upstream master?

@cdeil - do you want to try and open a new pull request with a rebased version of this branch?

Sorry @astrofrog , but I tried to rebase on upstream and I got a lot of conflicts. Unfortunately I don't have a good developing environment for Astropy at the moment, so it is not easy to me solve all conflicts in the proper way. Is it really necessary that I do the rebase or even @cdeil can do it?

@lpaioro - we can see to doing it. Actually I'll do it right now.

@cdeil - I'm going to create a repository where we both have write access so we can make efficient progress on this.

@cdeil - I've rebased the SAMP branch here:

https://github.com/astrofrog/astropy-samp

I thought I could open a PR from that new repo but I can't because it's not a real fork. So just copy the branch from there, push it to your fork, and open a new pull request. I'll remove that repository once you've pushed it to your fork. It makes sense for you to open the PR since you will likely be the main person working on this.

@lpaioro - just in case you don't have time to help much over the next few months, could you send me and @cdeil an email confirming that you give us permission to use the SAMPy code under a BSD license? Thanks!

@cdeil - I have a little time now so I will open the new pull request and work on it a bit. For any changes, open a pull request to my branch.

This is continued in #1907