Implement high-level table and image SAMP routines #1920

astrofrog · 2014-02-07T18:37:45Z

Once #1907 is merged, it would be worth writing simple functions that can take a filename, a Table, or an NDData object and send them over SAMP (optionally to a specific client).

@taldcroft - one idea I had would be to add two methods to Table:

>>> t = Table(...)
>>> t.send_via_samp(...)

would make the table appear e.g. in TOPCAT or DS9, and:

>>> t.receive_from_samp(...)

would be a class method that would 'hang' until a table is received via SAMP. The same could be done for NDData.

cc @cdeil @keflavich

taldcroft · 2013-12-20T04:23:24Z

Yes, that would be really good! Instead of new methods could we re-use Table.read and write with format='SAMP'? Read/write via SAMP is just a natural extension of read/write to a file (handle), no?

astrofrog · 2013-12-20T07:36:24Z

@taldcroft - this is another option, though I'm not sure if the current infrastructure would support it since I think we may have assumed there is always at least one argument passed. I'll look into it.

astrofrog · 2014-02-07T18:52:40Z

@taldcroft - for information, I just made this into a pull request. See the new 'Getting started' section here:

http://astrofrog-debug.readthedocs.org/en/latest/vo/samp/index.html

The NDData functionality is incomplete since it relies on having FITS readers/writers for NDData (which are not yet implemented but which I just implemented in #2063.

This is just an idea so far, and I'm open to suggestions if you think there is a better way to do this.

cc @cdeil @embray @eteq @mdboom @pllim

pllim · 2014-02-10T15:00:38Z

I am not familiar with SAMP but this looks pretty neat!

mdboom · 2014-02-10T16:43:34Z

docs/vo/samp/example_single_client.rst

+          :func:`~astropy.vo.samp.high_level.send` and
+          :func:`~astropy.vo.samp.high_level.receive` functions in
+          `astropy.vo.samp`, but the following sections describe in detail how
+          to do with using the various classes from the object-oriented API.


I think the content of the note needs to begin on the next line and be intended. This looks funny in the generated output.

…ive tables.

…', 'aladin', and 'topcat' destinations.

…ctionary. Added new 'Getting started' section for documentation.

embray · 2014-02-11T22:49:45Z

astropy/vo/samp/high_level.py

+    while not (hasattr(object, 'received') and object.received):
+        time.sleep(step)
+        if timeout is not None and time.time() - start_time > timeout:
+            raise AttributeError("Timeout while waiting for message to be received")


I see how this is waiting on a received attribute to show up, but raising an AttributeError here wouldn't be appropriate either. It should maybe be some type of IOError or other EnvironmentError subtype.

Maybe a TimeoutError?

eteq · 2014-02-12T00:27:56Z

This looks pretty neat...

I'm not clear on the limits of this, though: can I use this to do many various things with e.g., DS9, along the lines of what XPA allows? Or only sending images?

astrofrog · 2014-02-12T08:50:40Z

@eteq - this is for sending data objects, specifically tables and images. I haven't explored everything that SAMP allows yet, so I'm not sure how much more interaction can be done via SAMP.

mbtaylor · 2014-02-12T10:10:32Z

SAMP is a general purpose RPC mechanism, so what you can do depends on the RPC calls ("MTypes" in SAMP terminology) exposed by individual clients. There is a shortish list of the most commonly-used MTypes at http://wiki.ivoa.net/twiki/bin/view/IVOA/SampMTypes; this PR uses a couple of those to make it very easy to do a couple of commonly required things (send table and image).

Using the lower level SAMP API you can do whatever clients allow. In the case of ds9, yes this does let you do all the things you can do from XPA, using the custom ds9.get and ds9.set MTypes, e.g.:

from astropy.vo.samp import SAMPIntegratedClient
client = SAMPIntegratedClient()
client.connect()
params = {}
params["cmd"] = "cmap Heat"
message = {}
message["samp.mtype"] = "ds9.set"
message["samp.params"] = params
client.notify_all(message)

to select the "Heat" colour map. This is documented in the ds9 manual.

With that in mind - although this new functionality is very nice for doing what it does, it might be nice to add a short note in the documentation indicating that SAMP can do more things than send/receive tables and images. The current description looks a bit like sending tables and images is what it's for, and you can either do it the easy way or the masochistic way. If you'd like me to draft or review a comment to that effect I can do.

astrofrog · 2014-02-16T13:49:01Z

@mbtaylor - if you could write a small paragraph that I can insert at the start of the documentation, that would be great!

mbtaylor · 2014-02-17T11:21:32Z

Here is a stab at a paragraph giving some wider context (though it might make more after the send/receive documentation rather than before it):

The send() and receive() functions provide easy ways to exchange images and tables between applications, which are two of the most common tasks for SAMP. However, SAMP is a general purpose mechanism for exchanging data or control between participating clients, and it supports a range of other operations. To perform these, send a message using the low-level SAMP client classes with an "MType" (message type) that specifies the operation. A list of well-known MTypes for operations like exchanging spectra, communicating sky positions, and marking table rows can be found near http://www.ivoa.net/samp/. Clients are also free to define custom MTypes, such as ds9.set/ds9.get, used for full external control of SAOImage ds9.

The actual URL for the MType list is currently http://wiki.ivoa.net/twiki/bin/view/IVOA/SampMTypes; I've given a URL one level up from there in this text because it's shorter, and it should be permanent, while the wiki one is subject to change if the wiki hosting arrangements change.

Also: In the section "Using a SAMP client to communicate with other clients" the details of the arguments you have to supply look a bit like magic. To explain where this all comes from, it might be a good idea to provide pointers to two things:

the MType list as above, which specifies semantics as well as arguments for each well-known MType
the SAMP standard document (http://www.ivoa.net/documents/SAMP/20120411/ for current version), which explains the details of the arguments for the client API calls (the places to start reading are sections 3.11 and 3.12)

keflavich · 2014-02-17T18:11:07Z

I tried sending an image from ds9 and got the following. It was a slice from a data cube, not sure if that matters.

data = receive()
Traceback (most recent call last):
  File "<ipython-input-3-976d384c1a0d>", line 1, in <module>
    data = receive()
  File "/Users/adam/repos/astropy/astropy/vo/samp/high_level.py", line 116, in receive
    data = NDData.read(r.params['url'])
  File "/Users/adam/repos/astropy/astropy/io/registry.py", line 316, in read
    'read', cls, path, fileobj, args, kwargs)
  File "/Users/adam/repos/astropy/astropy/io/registry.py", line 378, in _get_valid_format
    "{0}".format(format_table_str))
Exception: Format could not be identified.
The available formats are:
Format Read Write Auto-identify
------ ---- ----- -------------

astrofrog · 2014-02-17T18:25:03Z

@keflavich - ah yes, this is due to the fact I'm relying on an NDData reader to be present :) (see #2063). Receiving tables should work though, and I can always modify the code here to just use fits.open instead. Was just thinking ahead!

astrofrog · 2014-02-17T18:28:46Z

@keflavich - try again, it should work now!

astrofrog · 2014-02-18T20:06:08Z

@mbtaylor - thanks! I've added the text you suggested and some extra details about MTypes in the 'communicating with other clients' section.

astrofrog · 2014-02-18T22:55:41Z

@taldcroft - just pinging you in case you hadn't seen this yet since maybe you would be interested in this from the Table point of view.

@keflavich - can you confirm that it all works now?

Any other comments or objections to merging?

eteq · 2014-02-27T16:50:56Z

@taldcroft @mbtaylor @astrofrog : I'm fine with requiring the destination be specified, or with using the enotify methodology. But I'm trying to approach this from the "typical astronomer user" perspective - my perception of the goal of this "high-level" scheme is to make it so that user's don't have to understand SAMP in any detail to be able to do fairly common tasks.

So I'm fine with forcing users to specify a destination, as I don't think it'll be a shock to a user that they have to tell their interface somewhere that they're connecting to ds9. But anything that includes non-inuitive bits like ecall, "c3", or even samp.mtype is problematic, because it's not clear what those things mean if you don't know SAMP very well.

I agree this isn't something to be done in general, if there's not a standard, but I think it's fair to include some convenience functionality for something like ds9 which is sort of a defacto standard because of how many people use it.

taldcroft · 2014-02-27T17:44:09Z

@eteq - I didn't mean to imply the destination is always required, just that it shouldn't be auto-inferred. If not supplied then the message will be broadcast for any listeners of that MType.

In general I would think users would be comfortable with providing a destination like ds9 or aladin, and in fact I find it confusing to just broadcast a table to anyone who might be listening. For instance if I start ds9 before I run the example in the documentation here, then ds9 also grabs the table and pops up a source catalog window. This is obviously not the intended behavior, so it would be natural (but not required) to normally specify the destination.

I completely agree that this high-level interface should shield the user from the details of SAMP. At the same time I think it would be good if it actually has the flexibility to allow taking full advantage of documented SAMP interfaces like http://ds9.si.edu/doc/ref/samp.html. That's why I like the suggestion of putting in some logic internally to do the connections and decide between notify vs. call_and_wait. At first glance that seems like it would require some hardwiring, e.g. to know that ds9.get should do a call and wait. Personally I'm not against a bit of hardwiring for the most common applications, practicality beats purity and all.

astrofrog · 2014-02-27T20:45:12Z

Just to add to what's been said already - this is a proposed high-level API, but all the basic OO API for SAMP will still be there. So it's just a matter of making the documentation clear so both impatient users and developers can make use of what they need.

I'm +1 on @mbtaylor's idea of:

def send(mtype, notify=False, destination=None, **params)

and then send_table and send_image. If there are no objections, I'll add this here. I'll also add a note that if users want to do more than a couple of send calls, they really should use the lower-level API if they care about performance, otherwise a client is started up and killed every time.

I'm also wondering whether we should consider switching enotify_all to notify_all (and rename notify_all to something else) if the former is more Pythonic and what most people will use. In fact, one could even ask if the current notify_all is needed at all, since it seems both developer and user-friendly to me. Or are there use cases where the current notify_all can do something enotify_all can't?

eteq · 2014-02-27T20:57:44Z

@astrofrog's suggestion seems good to me - I agree that having both enotify_all and notify_all will likely be confusing (for example, I still don't understand the distinction...) so @astrofrog's suggestion seems like a good way to get around it.

So @taldcroft, was your suggestion of logic to decide between notify vs. call_and_wait a suggestion you want to incorporate inside send (as part of @astrofrog's suggestion), or as part of the OO "lower-level" framework? (I think you meant the former, but just want to make sure)

astrofrog · 2014-02-27T21:05:42Z

By the way, we can also work on simplifying the lower-level API to make it more accessible to users. For example, you could imagine:

with SAMPClient() as client:
    client.notify_all('ds9.set', cmd="cmap Heat")

If that worked, maybe a higher-level API wouldn't be needed there? It would also avoid repeated connect/disconnect.

I still like send_table and send_image because they do abstract away some other issues (like transforming the object to a filename URL, etc.)

EDIT: although the 'nice' way of specifying the destination doesn't exist in the lower API.

taldcroft · 2014-02-27T21:35:42Z

So @taldcroft, was your suggestion of logic to decide between notify vs. call_and_wait a suggestion you want to incorporate inside send (as part of @astrofrog's suggestion), or as part of the OO "lower-level" framework? (I think you meant the former, but just want to make sure)

I meant as part of send.

But looking more closely at MessageSender I think I now understand the original suggestion better. The notify=False didn't mean much initially, but I think we can (and should) avoid having logic in send with this parameter, but renamed to something obvious and friendly like get_response=False. That puts a small burden on the user to tell send that they are sending a message that requires a response, but it will make the code cleaner and much more explicit:

 response = send('ds9.get', cmd='cmap', get_response=True, ...)  # waits for response
 send('ds9.set', 'cmap heat')  # no response

mbtaylor · 2014-02-27T22:38:40Z

I'll try to address some of the points mentioned in the last few messages.

Unpythonic methods:
The SAMPIntegratedClient methods notify, notify_all, call_and_wait etc exist and are so named because they and their arguments map one-to-one to the calls in the abstract API defined by the SAMP protocol (see sec 3.11), and hence the XML-RPC messages on the wire. There is no requirement to expose such one-to-one mappings in any given language-specific API that provides SAMP functionality. However, it can be useful, for instance if a SAMP expert wants to see what's going on, somebody is trying to port some SAMP code from a different language, or you want to send a pathologically illegal message for some perverse reason. Since these methods are already there, it would seem like a shame to delete them altogether. But if you want to hide them from the casual user in some way (e.g. rename them raw_notify_all etc, or make them methods on a class that is not generally promoted for everyday use), I think that would be quite reasonable.

Specifying whether a response is required or not:
It's not the case that some MTypes require responses and others don't --- it's just down to the user sending the message whether they want to see a response from that particular invocation or not. So it makes perfect sense for a get_response flag to be specified by the user at call time, rather than for it to be guessed by the method examining the MType. It's quite legal to call ds9.get as a notify-type message (no response received) though it may not achieve much in practice.

Instead of (variation on) my original suggestion of

response = send('ds9.get', cmd='cmap', get_response=True, ...)  # waits for response
send('ds9.set', 'cmap heat')  # no response

an alternative would be to indicate the difference with different method names. Following the usual SAMP terminology (optional) you could instead have:

response = call('ds9.get', cmd='cmap', ...)  # waits for response
notify('ds9.set', 'cmap heat')  # no response

Note also that "response" here covers error conditions as well as return values. If you use the notify delivery pattern, you have no way of knowing what, if anything, happened at the other end. If you use call, you get (apart from any other return value payload) a status response indicating whether the receiving client processed the message successfully. It would make sense for a medium- or high-level python method to present error responses by raising exceptions (and indicate success by not raising an exception).

"Nice" way of specifying the destination:

If you want to specify a target client by name ("ds9") rather than by opaque client ID ("cli#4") you have to do more than rearrange the arguments. Essentially it's necessary to query the hub for the available clients and their metadata and examine each samp.name metadata item to find one which matches the one you're after, and use the client ID associated with that, like the list_clients method does (at least, that's the easiest way to do it). I agree there should be API available to shield the user from this effort, but be aware that the communications need to be done to support it. One way to address that that would be to provide a method get_id_for_name, so you could call

ds9_id = c.get_id_for_name('ds9')
c.enotify(mtype='ds9.get', recipient_id=ds9_id, cmd='cmap heat')

If you are sending multiple messages to the same recipient, you then don't need to do the ID determination multiple times. Note the get_id_for_name call could fail if there is not exactly one client reporting the name 'ds9'.

mbtaylor · 2014-02-28T16:16:31Z

BTW the source code for MessageSender might help with understanding the logic and how to implement it: https://github.com/mbtaylor/jsamp/blob/master/src/java/org/astrogrid/samp/test/MessageSender.java.
Just pull the curly brackets out and it's practially python :-)

@taldcroft

This allows external clients to get a URL for one of the tables currently loaded into topcat. This functionality was suggested by @taldcroft in the discussion astropy/astropy#1920. The MType (table.get.stil) resembles the existing table.load.stil. Possibly there should be a table.get.votable etc, but that should probably be decided with consultation from the SAMP community. The machinery is present to add those - it would be a one-liner.

mentioned in astropy#1920.

astrofrog · 2014-05-18T15:24:43Z

This requires some more work, so I'm going to re-schedule for 1.0

mentioned in astropy#1920.

@taldcroft

This allows external clients to get a URL for one of the tables currently loaded into topcat. This functionality was suggested by @taldcroft in the discussion astropy/astropy#1920. The MType (table.get.stil) resembles the existing table.load.stil. Possibly there should be a table.get.votable etc, but that should probably be decided with consultation from the SAMP community. The machinery is present to add those - it would be a one-liner.

astrofrog · 2015-01-15T10:21:32Z

Removing milestone since I haven't had time to work on it for 1.0.

pllim · 2017-06-15T20:15:33Z

This is conflicts galore now. Should we close this?

astropy-bot · 2017-09-28T12:33:52Z

Hi humans 👋 - this pull request hasn't had any new commits for approximately 3 years. I plan to close this in a month if the pull request doesn't have any new commits by then.

In lieu of a stalled pull request, please close this and open an issue instead to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary.

If you believe I commented on this issue incorrectly, please report this here.

astropy-bot · 2017-11-21T11:13:54Z

⏰ Time's up! ⏰

I'm going to close this pull request as per my previous message. If you think what is being added/fixed here is still important, please remember to open an issue to keep track of it. Thanks!

If this is the first time I am commenting on this issue, or if you believe I closed this issue incorrectly, please report this here.

astrofrog mentioned this pull request Dec 19, 2013

Implementation of astropy.vo.samp from SAMPy #1907

Merged

9 tasks

astrofrog mentioned this pull request Jan 15, 2014

Add 'What's new' entry for astropy.vo.samp #1968

Closed

mdboom reviewed Feb 10, 2014
View reviewed changes

astrofrog added 7 commits February 11, 2014 17:59

Added initial code for high-level routines

ba4c457

Added functions to list the tables and list the clients that can rece…

53c314b

…ive tables.

Generalize high-level SAMP interface

a4734cf

More improvements to high-level interface, including support for 'ds9…

6e46d12

…', 'aladin', and 'topcat' destinations.

Make list_clients() return summary table instead of unreadable di…

3a1110a

…ctionary. Added new 'Getting started' section for documentation.

Added initial tests for high-level interface

7db92ef

Minor ResT fixes suggested by @mdboom

91ec845

embray reviewed Feb 11, 2014
View reviewed changes

Use fits.open instead of NDData.read (until the latter is implemented)

0ceff2b

Added more details about MTypes and messages as suggested by @mbtaylor

07973ab

astrofrog added 2 commits February 18, 2014 21:22

Use a TimeoutError in the high-level SAMP API

c7a6e7d

PEP8

7f154f3

mdmueller pushed a commit to mdmueller/astropy that referenced this pull request Apr 2, 2014

Preserve all floating-point precision when writing out a votable. As

a5f9f06

mentioned in astropy#1920.

astrofrog self-assigned this May 3, 2014

astrofrog modified the milestones: v1.0.0, v0.4.0 May 18, 2014

embray force-pushed the master branch from cd9dfdf to 5d7acd6 Compare August 29, 2014 21:15

ktchrn pushed a commit to ktchrn/astropy that referenced this pull request Oct 28, 2014

Preserve all floating-point precision when writing out a votable. As

85a5700

mentioned in astropy#1920.

astrofrog assigned astrofrog and unassigned astrofrog Nov 5, 2014

astrofrog removed this from the v1.0.0 milestone Jan 15, 2015

embray force-pushed the master branch from 63540e0 to 34f4c29 Compare October 5, 2015 21:18

astrofrog moved this from Pending classification to Low priority in 2.0 Feature Planning Feb 9, 2017

taldcroft moved this from Low priority to Postpone to future versions in 2.0 Feature Planning Jun 12, 2017

pllim added the Close? label Jun 15, 2017

astropy-bot bot closed this Nov 21, 2017

astrofrog added the closed-by-bot label Nov 21, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement high-level table and image SAMP routines #1920

Implement high-level table and image SAMP routines #1920

astrofrog commented Feb 7, 2014

taldcroft commented Dec 20, 2013

astrofrog commented Dec 20, 2013

astrofrog commented Feb 7, 2014

pllim commented Feb 10, 2014

mdboom Feb 10, 2014

astrofrog Feb 11, 2014

embray Feb 11, 2014

astrofrog Feb 12, 2014

astrofrog Feb 18, 2014

eteq commented Feb 12, 2014

astrofrog commented Feb 12, 2014

mbtaylor commented Feb 12, 2014

astrofrog commented Feb 16, 2014

mbtaylor commented Feb 17, 2014

keflavich commented Feb 17, 2014

astrofrog commented Feb 17, 2014

astrofrog commented Feb 17, 2014

astrofrog commented Feb 18, 2014

astrofrog commented Feb 18, 2014

eteq commented Feb 27, 2014

taldcroft commented Feb 27, 2014

astrofrog commented Feb 27, 2014

eteq commented Feb 27, 2014

astrofrog commented Feb 27, 2014

taldcroft commented Feb 27, 2014

mbtaylor commented Feb 27, 2014

mbtaylor commented Feb 28, 2014

astrofrog commented May 18, 2014

astrofrog commented Jan 15, 2015

pllim commented Jun 15, 2017

astropy-bot bot commented Sep 28, 2017

astropy-bot bot commented Nov 21, 2017

Implement high-level table and image SAMP routines #1920

Implement high-level table and image SAMP routines #1920

Conversation

astrofrog commented Feb 7, 2014

taldcroft commented Dec 20, 2013

astrofrog commented Dec 20, 2013

astrofrog commented Feb 7, 2014

pllim commented Feb 10, 2014

mdboom Feb 10, 2014

Choose a reason for hiding this comment

astrofrog Feb 11, 2014

Choose a reason for hiding this comment

embray Feb 11, 2014

Choose a reason for hiding this comment

astrofrog Feb 12, 2014

Choose a reason for hiding this comment

astrofrog Feb 18, 2014

Choose a reason for hiding this comment

eteq commented Feb 12, 2014

astrofrog commented Feb 12, 2014

mbtaylor commented Feb 12, 2014

astrofrog commented Feb 16, 2014

mbtaylor commented Feb 17, 2014

keflavich commented Feb 17, 2014

astrofrog commented Feb 17, 2014

astrofrog commented Feb 17, 2014

astrofrog commented Feb 18, 2014

astrofrog commented Feb 18, 2014

eteq commented Feb 27, 2014

taldcroft commented Feb 27, 2014

astrofrog commented Feb 27, 2014

eteq commented Feb 27, 2014

astrofrog commented Feb 27, 2014

taldcroft commented Feb 27, 2014

mbtaylor commented Feb 27, 2014

mbtaylor commented Feb 28, 2014

astrofrog commented May 18, 2014

astrofrog commented Jan 15, 2015

pllim commented Jun 15, 2017

astropy-bot bot commented Sep 28, 2017

astropy-bot bot commented Nov 21, 2017