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

repetition of null value bug #111

Closed
frankeye opened this issue Mar 26, 2020 · 15 comments
Closed

repetition of null value bug #111

frankeye opened this issue Mar 26, 2020 · 15 comments

Comments

@frankeye
Copy link

Hi Marshall,
Thanks for this great tool!

The bug occurs for the following variable input line:
AXFCLN = 3.0, 3.0, 48*,
The 48* denotes a repetition of 48 null values
Upon .write(), it is being turned into:
AXFCLN = 3.0, 3.0, ','*,

One clue is that everything is fine (output = input) if the trailing comma is removed prior to processing.

Processing attached files as follows:
nml=f90nml.read('repetition_of_null_win.nml')
f90nml.patch('repetition_of_null_win.nml', nml, 'patched_data_win.nml')
and likewise for the unix line-endings version.

I'm using latest master commit 151438f on Oct 31, 2019 (ver 1.1.2)
My sys.version is:
3.7.5 (tags/v3.7.5:5c02a39a0b, Oct 15 2019, 00:11:34) [MSC v.1916 64 bit (AMD64)]
I'm running Win10/64/Pro.

patched_data_unix.txt
patched_data_win.txt
repetition_of_null_win.txt
repetition_of_null_unix.txt

@marshallward
Copy link
Owner

Yes, I can also reproduce this problem. Looks like the failure is at read() and the comma is being repeated rather than the null value. Removing the comma gives the expected value.

I will look further into it, thanks very much for reporting.

@marshallward
Copy link
Owner

I've pushed a fix 7fd2abf that appears to resolve this, and have added your case as a test. Can you see if it works?

@frankeye
Copy link
Author

patched_data_win.txt
Hi Marshall,
Thanks!
It's not quite working on my end. Patched output is missing the trailing comma on 1st variable entry, then misconstruing the following instance of repeated-null. Weirdly, the subsequent instance is handled correctly.

@marshallward
Copy link
Owner

Yes, you are right. There was a second assumption in the lookahead when checking the subsequent record. I fixed one case (the final value) but not the other (first or interior value).

Sorry about the confusion, let me have another go.

@frankeye
Copy link
Author

frankeye commented Mar 27, 2020 via email

@marshallward
Copy link
Owner

I've done a more careful look into this case (repeat nulls with commas) and it seems to be fixed now. I've also expanded the test suite to catch more of these cases.

I've updated this in 3096833. Can you give it another try?

@frankeye
Copy link
Author

Hi Marshall,
Almost...! Only trouble now is the patch output is missing the trailing comma on the entry just above the first instance of repeated null.

@marshallward
Copy link
Owner

I can't see the issue that you're describing, can you post the output here?

@frankeye
Copy link
Author

Sure (my apologies):
(This happens both for win and unix flavors of files)

                           Repetition of Null Namelist:
                           ----------------------------

$REDOIO
OUTPUT = 'sample.out'
AXFCLN = 3.0, 3.0, 48*,
RDFCLN = 3.0, 3.0, 48*,
$END

@marshallward
Copy link
Owner

marshallward commented Mar 27, 2020

Ah ok, I think that may be intentional. I need to run but default behavior is to not print trailing commas.
There is a flag to append them, but will have to reply a bit later on how to enable this. (If you're feeling ambitious, you can trawl through the documentation.)

I would not have thought this affected patched output, but this may be what is going on.

(Edit: Null values are one of the exceptions which always append a comma. Although this is not strictly necessary when they are repeated, it may just be that the program is still using them.)

@frankeye
Copy link
Author

Thanks, I came across the .end_comma and such object members, but recorded that these seemed to have no impact on .patch() output. I tried via first:
nml.end_comma=True
then invoking .patch().

@marshallward
Copy link
Owner

You are right, that flag only affects write() output, and wouldn't be the cause of this issue.

Unfortunately I don't have an easy explanation here. The mechanics of patch() are unusual in that they catch and modify tokens as they are being read, and then the tokens are streamed back into a new output.

My guess is that commas are being gathered along with the values, since in may cases they are needed, but is discarding them because they are not part of the output.

I agree that it should reproduce the commas if they are present, but it could be a large task and wanted to give some warning.

@marshallward
Copy link
Owner

I've confirmed this is a general problem, gonna recommend closing this issue and opening a new one for the trailing comma of a patch.

@frankeye
Copy link
Author

frankeye commented Mar 27, 2020 via email

@marshallward
Copy link
Owner

Closing this for now, the end comma issue will be addressed in #112.

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

2 participants