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

6S-V2.1 support #32

Open
vincentschut opened this issue Feb 21, 2017 · 3 comments
Open

6S-V2.1 support #32

vincentschut opened this issue Feb 21, 2017 · 3 comments

Comments

@vincentschut
Copy link
Contributor

What would be involved to add support for 6S-V2.1?

This is related to #31

Is it as simple as, depending on the version of 6S (available in the first line of the output of 6S), just skipping the extra 2 lines (see example in #31)?

@jason-tilley
Copy link

I too have this question. It seems you might be correct that skipping those lines would do the trick. I changed all occurrences of '1.1' to '2.1' in sixs.py (3 times), and commented out the following in outputs.py (lines 61–64):

    if len(stderr) > 0:
            # Something on standard error - so there's been an error
            print(stderr)
            raise OutputParsingError("6S returned an error (shown above) - check for invalid parameter inputs")

After doing this I get the following output from SixS.test():

6S wrapper script by Robin Wilson
Using 6S located at /opt/local/bin/sixsV2.1
Running 6S using a set of test parameters
6sV version: 2.1
The results are:
Expected result: 619.158000
Actual result: 619.158000
#### Results agree, Py6S is working correctly
0

Obviously, this isn't a fix, because you wouldn't want to disable the error checking there. But it does seem that it may be as simple as you propose.

@lukasValentin
Copy link

I am not sure if this issue is still of relevance; with the latest release of Py6S, changing the version number from 1.1 to 2.1 seems to do the trick - without any further modifications. Since I am not really an expert regarding SixS, however, I am not sure if there are not any unintended side-effects. Maybe the developer team can answer this question?

@dobedobedo
Copy link

I confirmed with Dr Wilson, and he said it's not officially supported yet. It might work, but no guarantee. Better to wait until the official announcement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants