failure when checking HDUlist type

[joemasiero]

Hi, just got a new mac (running latest Sierra, 10.12.6) and doing a fresh install of my python tools. Installed python 2.7.14, pyfits 3.4, pyds9 1.8.1. When I try to run:

inf=pyfits.open("S20170728S0023_bf.fits") dim=pyds9.DS9() dim.set_pyfits(inf)

I get an error:

ValueError: requires pyfits.HDUList as input

it appears to originate in pyds9.py line 644. Changing that line to: if type(hdul).name != "HDUList": seems to fix my problem, though may cause issues for astropy loads potentially (I haven't checked that yet).

There is also an error with XPA I think. When I start pyds9.DS9() it's fine if a ds9 window is already open, but it throws this error when one isn't:

inf=pyfits.open("S20170728S0023_bf.fits") dim=pyds9.DS9()

open: invalid option -- i

then prints a usage statement for 'open'. But that might be because of how I set up DS9. I'll poke around and see if I can find the issue to this as well.

[joemasiero]

I found a solution to the other problem I mention above. pyds9 line 71 reads:

if ds9Globals["ulist"][0] == 'Darwin' and not ds9[0]:

however because I put together a terrible command-line hack (a shell script in my bin/ that calls the ds9 aqua app), pyds9 was trying to call a new ds9 instance through my shell script, instead of falling back on calling the application directly as the following lines of pyds9 set up. If that line is changed to:

if ds9Globals["ulist"][0] == 'Darwin':

it seems to work fine. Again, it might just be my terrible hack, so I don't know if this change needs to be implemented for the distribution, or only just noted here so others can find it if needed.

[ericmandel]

I'll leave it to @montefra to decide whether:

if type(hdul).name != "HDUList":

is valid for all configurations.

The other change:

if ds9Globals["ulist"][0] == 'Darwin' and not ds9[0]:

to:

if ds9Globals["ulist"][0] == 'Darwin':

doesn't really work for the pyds9 test being made, although you are raising a real problem: the find_executables() routine is finding anything with the executable bit set: user shell scripts called "ds9" (which is your case, but which we don't really want to find) and the real ds9 binary program (which we do want to find). If pyds9 finds your ds9 shell script, it tries to run it, which gives an error. We need to distinguish between shell scripts and proper executables when looking for ds9.

[joemasiero]

My shell script was inspired by similar scripts I had seen online, made by other DS9 users trying to solve the same problem (i.e. getting command-line-like access to the Aqua DS9 install), so although this won't be a universal problem, it won't be isolated to only me. I'm not sure how you would test for a ds9 binary instead of a ds9 shell, other than trying to run it and failing over into the Aqua call, but that's kinda ugly.

I note that previous versions of pyds9 (back when 'import ds9' was the call) did correctly execute through the shell script, but that could be because of a change in python 2.7, not in pyds9.

[ericmandel]

I'm not a Python expert, but in principle one would simply do the equivalent of the "file" command in the shell to make the differentiation (or even run the 'file' command itself):

eric% file /Users/eric/bin/ds9
/Users/eric/bin/ds9: Mach-O 64-bit executable x86_64

eric% cat ds9
#!/bin/bash
echo "ds9 shell script"

eric% file ds9
ds9: Bourne-Again shell script text executable, ASCII text

We'll have to wait for @montefra to weigh in on what ought to be done in Python, not in principle.

[montefra]

@joemasiero

thank you for the report

it appears to originate in pyds9.py line 644. Changing that line to: if type(hdul).name != "HDUList": seems to fix my problem,

Can you please add the following line before the line and give me the output?

print(type(hdul), isinstance(hdul, pyfits.HDUList))

I tried to install pyfits on my machine but fails with a recursion error from distutils and I can't find how to install it in anaconda. If it works, we should use instance instead of matching class names or types.

[montefra]

@joemasiero: now `pyds9 If that line is changed to:

if ds9Globals["ulist"][0] == 'Darwin': it seems to work fine.` tries to find a ds9 executable and then, if doesn't find it and is on a Mac, it looks for the DS9 app.

If that line is changed to:

if ds9Globals["ulist"][0] == 'Darwin': it seems to work fine.

I don't like this solution because would force Mac users to use the DS9 app, instead of the x11 based execution and would break pyds9 for a lot of people.

@ericmandel:

in principle one would simply do the equivalent of the "file" command in the shell to make the differentiation (or even run the 'file' command itself)

I don't like this either. It looks very brittle to me. If we go along this line we would need to parse the output and match against a long list of strings for various platforms, versions, distros. And the next OSX or linux will break it, because someone decides to change the output of file.

I see two options:

1. On a Mac first look for the DS9 app and, if not found, search an executable
2. We add an environmental variable that forces using the DS9 app on Mac.

I am not a Mac user, so I don't know which option would be better and less destructive for users.

[ericmandel]

OK, that's fine ... one could test the first two characters of the file content for "#!", which always indicates the presence of a shell script to the program loader. But I won't insist ...

We add an environmental variable that forces using the DS9 app on Mac.

A DS9_EXE environment variable (as well as XPANS_EXE) that (when present) is used to set the ds9 and xpans arrays (instead of calling find_executable) is a good idea. Allowing it to be set explicitly even gives the user full control over which version of ds9 to start up on a Mac, in cases where both X11 and Aqua are installed.

On a Mac first look for the DS9 app and, if not found, search an executable

This is probably less good, as it forces use of the Aqua version (if found), and still does not distinguish between scripts and programs (e.g. Chandra CIAO environment actually supplies both!)

[joemasiero]

@montefra

When I put in:

print(type(hdul), isinstance(hdul, pyfits.HDUList))

This is the output I get:

<class 'pyfits.hdu.hdulist.HDUList'> False

I was trying to use a type comparison for another code I'm working on, but was getting behavior different from what I was expecting and what example codes were suggesting online, so I don't know if there's been some subtle change in comparing types in the latest python, or if this a change in pyfits, or what.

I got pyfits from pip.

As to the executable, the simplest install for Mac users is to use the Aqua version, though it doesn't offer command line interface. So, you have to either make the shell script work around, or build it from source. But either way, it's likely that the user will have the App installed, and probably won't uninstall it. But since it functions the same, checking for the app first and then falling back on the executable should be fine. Unless the user only updates one of the two.

I'm a bit hesitant about the environment variable solution, only because it's another manual step and thus another chance for a buggy install.

Again, the current scheme worked for me on my previous laptop with older versions of python, etc, so I don't know if this is necessarily an error associated with pyds9, or if something in the latest update of python changed the way subprocesses are called that is breaking the shell script. The error I get looks to be from inside the shell script when the arguments are getting passed through the shell to the Application.

[ericmandel]

Yes, I don't see why your ds9 script should not work. Can you paste its contents? Mine does the following on my 10.12.6 Mac (Python 2.7.10), and it works as expected:

#!/bin/bash
open -a "SAOimage DS9" --args $*

I'd hate to think an upgrade from 2.7.10 to 2.7.14 broke something.

[ericmandel]

OK, I upgraded to 2.7.14 and the above script still works as expected ...

[joemasiero]

Here's my ds9 script:

#!/bin/sh
open /Applications/SAOImage\ DS9.app $@

The error was complaining about an invalid option 'i' so I don't know if perhaps something is getting parsed strangly by my shell script

[ericmandel]

Ah, I see, in your script you need to add --args before the $@:

open /Applications/SAOImage\ DS9.app --args $@

so that open knows to pass command line arguments properly. You can see the error happen in the shell by simply executing:

ds9 -title foo

Not sure if we did something to make this problem rise to the surface but the fix is easy ...

[joemasiero]

Ha, ok. Thanks for the help on that. I guess I've just been throwing fits files at the command line, so never really had argument issues before.

Yup, seems to work fine now. Thanks again

[ericmandel]

OK, great ... one down, but we still have to deal with the check on the HDUList type before we can close this.

[montefra]

@ericmandel @joemasiero : I'm happy to see that you fixed the script issue.

I'll think some more about how to handle the HDUList type comparison.

@joemasiero :

print(type(hdul), isinstance(hdul, pyfits.HDUList))
<class 'pyfits.hdu.hdulist.HDUList'> False

This is weird. I'll try again to install pyfits and to check the problem. Let's see if I manage to find a solution soon.

[montefra]

@joemasiero I have investigate a bit and I think that I figured out the problem:

• if both astropy and pyfits are present on the system, pyds9 uses astropy.
• DS9().set_pyfits check that the input is a fits object from astropy or pyfits, whichever is imported
• thus, if you have astropy and you pass a pyfits fit, you get the error. And I think that the error is missleading.

The current pyds9 master requires astropy. I am going to modify it to allow to use both astropy and pyfits objects.

[montefra]

@joemasiero : I hust merged PR #56 that allows to use astropy and pyfits fits objects independently adding {set,get}_fits methods to handle astropy fits. This should make more clear the workflow and use both astropy and pyfits objects, also at the same time. I belive that this will either fix your problem or give you a more sensible error message.

If you want to test, you are welcome to do so :smile:

[joemasiero]

Thanks Francesco. I'll check it out.

Joe

On Fri, Nov 10, 2017 at 6:55 AM, Francesco Montesano < notifications@github.com> wrote:

@joemasiero https://github.com/joemasiero : I hust merged PR #56 https://github.com/ericmandel/pyds9/pull/56 that allows to use astropy and pyfits fits objects independently adding {set,get}_fits methods to handle astropy fits. This should make more clear the workflow and use both astropy and pyfits objects, also at the same time. I belive that this will either fix your problem or give you a more sensible error message.

If you want to test, you are welcome to do so 😄

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ericmandel/pyds9/issues/51#issuecomment-343494397, or mute the thread https://github.com/notifications/unsubscribe-auth/AZsG7xYEhPAvaY19J2yIYWUeq9vkMLNCks5s1GP8gaJpZM4PxFig .

[montefra]

Issue solved by #56. Closing it