Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Potential compatibility changes for the xonsh shell #301

Open
DBerke opened this issue Oct 30, 2021 · 0 comments
Open

Potential compatibility changes for the xonsh shell #301

DBerke opened this issue Oct 30, 2021 · 0 comments
Assignees
Labels
compatibility Compatibility issues. eg with Python, dependencies like astropy, IRAF or other software usage Usage and user interfaces

Comments

@DBerke
Copy link
Contributor

DBerke commented Oct 30, 2021

While working through the DRAGONS tutorials, I discovered an issue where some operations failed in the xonsh shell. xonsh is a "Python-powered, cross-platform, Unix-gazing shell language and command prompt", which offer a mostly "Bash-wards" compatible experience while also allowing the use of Python code directly in the shell. It's among the shells conda is compatible with, so it's not unreasonable that DRAGONS users might use it (though how many actually do I've no idea).

The discrepancy happens due to xonsh's different handling of quoted arguments on the command line. In Bash quotes are essentially flags turning the interpreter on and off, while in xonsh they are considered part of the argument. For example, this command:
dataselect *.fits --expr='exposure_time==1'
produces a string value of exposure_time==1 for expr when run in Bash, and a value of 'exposure_time==1' when run in xonsh. Note the quotes in the latter; these quotes then cause expr_parser in dataselect.py to return a malformed string, which causes evalexpression to exit with an error.

There are a few potential ways to deal with this issue. The first (and simplest) is just to say that only Bash is officially supported for using DRAGONS. Without a lot of users using xonsh (or any other shells that might run into this issue), this is certainly an option. A second easy change is a documentation change: xonsh actually has a way to work around the issue using its own unique syntax, by wrapping a string in the @() notation which tells the interpreter to treat what's inside as Python code and execute it, e.g.: dataselect *.fits --expr=@('exposure_time==1'). This passes through the string without quotes, and works as expected. This fact could potentially be noted somewhere in the documentation for DRAGONS, and would allow people to use DRAGONS in xonsh with a minimum of effort on both sides.

A third option would be to strip quotation marks from strings entered where this issue would occur, in principle making DRAGONS function identically in xonsh and Bash (and potentially other shells with the same issue, though I don't know of any at present). Unfortunately a naive approach like applying .strip('"').strip("'") to expression strings runs into problems when the expression itself contains a string (potentially containing spaces) which needs quoting, such as 'object=="FS 17"'. A slightly clunkier solution might be to implement a 'quote stripping' function that could be used with argparse's type parameter for input verification, something like:

def strip_quotes(expr):
    if (expr[0] == '"') and expr[-1] == '"') or (expr[0] == '"') and expr[-1] == '"'):
        return expr[1:-1]
    return expr

Assuming such a function could be written that avoids any edge cases (see below), it could then simply be used for any interactive scripts taking input like so (from dataselect):

    parser.add_argument('--expr', type=strip_quotes, nargs=1,
                        dest='expression', action='store', required=False,
                        help='Expression to apply to descriptors (and tags)')

One thing I don't know at the moment is how many scripts would be affected. Another consideration is that before implementing such a change we'd want to have robust tests in place to try to find any currently-unforeseen edge cases. Thus at this point it's a question of whether supporting this quirk of xonsh's is worth the effort, when workarounds exist.

Edit: I've found a more serious case, where I can't get xonsh to work at all, in the GSAIO tutorial, with the command

disco `dataselect *_skyCorrected.fits --expr='observation_class=="partnerCal"'`

which exits with disco: error: No input files found. Nothing I've tried works, including piping the results of dataselect to disco (this also doesn't work in Bash), or putting the results of dataselect into a file and using the @-syntax to read it in to disco.

@DBerke DBerke added usage Usage and user interfaces compatibility Compatibility issues. eg with Python, dependencies like astropy, IRAF or other software labels Oct 30, 2021
@DBerke DBerke self-assigned this Oct 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility Compatibility issues. eg with Python, dependencies like astropy, IRAF or other software usage Usage and user interfaces
Projects
None yet
Development

No branches or pull requests

1 participant