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

token logic error? #42

Closed
greyltc opened this issue Feb 21, 2017 · 8 comments
Closed

token logic error? #42

greyltc opened this issue Feb 21, 2017 · 8 comments
Labels

Comments

@greyltc
Copy link

greyltc commented Feb 21, 2017

Hi. Thanks for your work here! When I read a file with the following contents

&column
   name=Humidity
   units=percent
   type=double
/

I get

Namelist([('column',
           Namelist([('name', 'Humidity'),
                     ('units', 'percent'),
                     ('type', 'double')]))])

as expected.

But when I read a file like this

&column
   name=Humidity
   units=%
   type=double
/

I get something very unexpected:

Namelist([('column',
           Namelist([('name', 'Humidity'),
                     ('units', None),
                     ('=', Namelist([('type', 'double')]))]))])

Is there something you can change to make this parse correctly?
I'm expecting this:

Namelist([('column',
           Namelist([('name', 'Humidity'),
                     ('units', '%'),
                     ('type', 'double')]))])

I guess the problem might be in this line or this line , but I'm not sure.
Thanks!

@marshallward
Copy link
Owner

marshallward commented Feb 21, 2017

Sure, that looks like a bug, I will try to look into it. Thanks for the report.

Things are quite busy for me workwise at the moment, and then I go on one-month leave, so it could potentially take quite a long time. In the meantime, you could try putting string delimiters around it ('%'), which seems to work for me.

@greyltc
Copy link
Author

greyltc commented Feb 21, 2017

Thanks for the quick response.
Yep, that's exactly what I'm doing for now.
I'm just a bit worried about other token combos biting me later!

@marshallward
Copy link
Owner

Following up on here.. (mostly for myself)

Problem appears to be L357 of parse_variable. We rely on the appearance of those tokens (%, (, =) to identify the end of the current variable and the beginning of the next variable. (This is primarily for when multiple variables, including vectors, are on a single line.)

This may be failing because it sees the % and thinks that it's finished, and then sets the value of units to None.

This will probably be a bit harder to fix. For example, this namelist:

&nml x=y%z=c /

could have two interpretations:

&nml
    x = 'y%'
    z = 'c'
/

&nml
    x = 
    y%z = 'c'
/

But there might be a way out of this particular situation. Your example only has one interpretation, since % would need a variable name to be interpreted as a derived type dereference.

@marshallward
Copy link
Owner

Actually I could be wrong about that example... spaces may be able to resolve the case that I mentioned, I'd need to check it out.

@marshallward marshallward mentioned this issue Mar 13, 2017
@marshallward
Copy link
Owner

Sorry for the long delay.

I've testd this case in gfortran and Intel fortran, and the results are similar to #43 in that gfortran rejects the namelist, and Intel fortran parses it differently (specifically it sets units to an empty string).

Given that these non-delimited tokens don't appear to produce the strings that you need, I'm probably not likely to support these.

From what you mentioned in #43, it may be that SDDS is producing very old namelist files that don't produce meaningful output on any actively maintained compiler, so I'm inclined to close these issues as not relevant. It might be more constructive to lean on SDDS to produce syntatically correct namelists rather than supporting incorrect namelists.

I could possibly support this case with a control flag or something that relaxes the parsing rules, but these are very hard edge cases to support, and I don't think I would be able to prioritise them.

@marshallward
Copy link
Owner

I'm going to close this ticket, due to the lack of support for these values in gfortran and Intel fortran, the only compilers available to me. If you can find a compiler which supports these values, or just feel like this is an urgent and unsolvable problem for you, then feel free to re-open this and we can talk about a plan. But I probably won't personally be able to devote much time to this one, sorry.

@marshallward marshallward changed the title token logic error? token logic error? (SDDS) Apr 28, 2017
@marshallward marshallward reopened this Apr 28, 2017
@marshallward
Copy link
Owner

Reopening, but explicitly noting this is an SDDS issue.

@marshallward marshallward changed the title token logic error? (SDDS) token logic error? Aug 2, 2017
@marshallward
Copy link
Owner

As in #43 this is probably too big a challenge to implement unless its strongly driven by user feedback, so will close for now. Please re-open in the future if necessary.

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

No branches or pull requests

2 participants