Feature/525 detect empty lines

[kordusas]

Pull Request Checklist for MontePy

Description

TODO

Fixes #525

---

General Checklist

• I have performed a self-review of my own code.
• The code follows the standards outlined in the development documentation.
• I have formatted my code with black version 25.
• I have added tests that prove my fix is effective or that my feature works (if applicable).

---

LLM Disclosure

Were any large language models (LLM or "AI") used in to generate any of this code?

• Yes
  • Model(s) used:
• No

Documentation Checklist

• I have documented all added classes and methods.
  - [ ] For infrastructure updates, I have updated the developer's guide.
  - [ ] For significant new features, I have added a section to the getting started guide.

---

Additional Notes for Reviewers

Ensure that:

• The submitted code is consistent with the merge checklist outlined here.
• The PR covers all relevant aspects according to the development guidelines.
• 100% coverage of the patch is achieved, or justification for a variance is given.

---

📚 Documentation preview 📚: https://montepy--819.org.readthedocs.build/en/819/

[kordusas]

so the test_importance_rewrite fails because of extra empty line added:

mode n p
vol NO 2J 1 1.5 J

imp:p 1 0.5 1 0 3 
imp:n 1 1
     1 0 3 

I assume this is not the expected behaviour? the importance should have been missing in this test?
I can correct accordingly

[MicahGale]

I assume this is not the expected behaviour? the importance should have been missing in this test?
I can correct accordingly

There's a list of "bad" input files that should be excluded from that test in tests/constants.py. I haven't reviewed this closely, but I think this new file is a good candidate for BAD_INPUTS?

[kordusas]

I assume this is not the expected behaviour? the importance should have been missing in this test?
I can correct accordingly

There's a list of "bad" input files that should be excluded from that test in tests/constants.py. I haven't reviewed this closely, but I think this new file is a good candidate for BAD_INPUTS?

Yes the new input is part of "BAD_INPUTS" and i have added it there, thank you.

I meant something else. the test_importance_rewritefails. The behavior seems odd — though perhaps intentional — so I wanted to confirm before making changes. Example below:

"""
Replicating importance rewrite issue
"""
import montepy
from io import StringIO

file="""Example Problem
1 0  -1 2 -3 IMP:n=1
2 0  -4 5 -6 IMP:n=1

1 CZ 0.5
2 PZ 0
3 PZ 1.5
4 CZ 0.500001
5 PZ 1.5001
6 PZ 2.0

kcode 1.0 100 25 100
TR1 0 0 1.0
TR2 0 0 1.00001
"""
# Create problem from string
problem = montepy.read_input(StringIO(file))
problem.print_in_data_block["imp"] = True

# Create a StringIO object to capture the output
output = StringIO()
problem.write_problem(output)

# Get the contents as a string
print(output.getvalue())

The output adds an extra blank line before the importances in the data block:

....
kcode 1.0 100 25 100
TR1 0 0 1.0
TR2 0 0 1.00001
                                            <-(empty line here)
IMP:n 1 1

[MicahGale]

Thanks for extracting that. That's definitely broken. So was this bug always in the tests, but now that warnings are being raised it's being flagged as an error? This might be related to #523? I think I might have seen this error before. Do you want to try and fix it, or open an issue and bug me to fix it?

I'm going to convert this to a draft while this is figured out.

[kordusas]

Thanks for extracting that. That's definitely broken. So was this bug always in the tests, but now that warnings are being raised it's being flagged as an error? This might be related to #523? I think I might have seen this error before. Do you want to try and fix it, or open an issue and bug me to fix it?

I'm going to convert this to a draft while this is figured out.

Sure no problem. I think it's been there all the time, just now it raises warning due to the extra line present.
I would like to adress this together with this PR if that is ok?

[MicahGale]

Sure no problem. I think it's been there all the time, just now it raises warning due to the extra line present.
I would like to address this together with this PR if that is ok?

Go for it. I just figured I probably created the bug, so it's not strictly your responsibility to fix it.

[MicahGale]

Thanks for getting back to this @kordusas. Just one slight concern for now.

Obviously we have discussed the test suite issues. Let me know if you have questions.

I ran coverage locally and confirmed that you have 100% patch coverage:

montepy/input_parser/input_syntax_reader.py     115      1  99.13%   181

[kordusas]

Sure no problem. I think it's been there all the time, just now it raises warning due to the extra line present.
I would like to address this together with this PR if that is ok?

Go for it. I just figured I probably created the bug, so it's not strictly your responsibility to fix it.

I think I fixed it, see if you approve.

[MicahGale]

I want to think more about how the line insertion error is handled. Also could you write up in the PR description

1. the feature added, and how it works.
2. The bug that is fixed, what is causing it, and how it was fixed.

Also CI failures is due to a 500 with coveralls that they are currently dealing with.

[MicahGale]

Some more thoughts; thanks for your patience. I'm still a bit skeptical with write_problem functioning as desired, but I may need to just see it work to be convinced.

[MicahGale]

I believe this will keep generating inputs until it hits a new block (which may never occur). I think it might be easiest to not have the flag, and rather just return right here? That may break some things, but the point would just terminate the generator early.

[kordusas]

I actually have tried adding return, but as you mentioned it breakes some things.

There was an error parsing "26056". sly: Syntax error at line 12, token=NUMBER

Adding return doesn’t stop the parser cleanly — it just fails instead of continuing.
Is that the preferred behavior, or should we handle this case more gracefully?
This would create a lot of extra errors for every line of text in the 5th block, making messages less readible. Users may have things there on purpose since MCNP just doesnt read it.

Regarding the comment about “keep generating inputs until it hits a new block” — I’m not sure I fully understand what you mean by this.

[MicahGale]

Regarding the comment about “keep generating inputs until it hits a new block” — I’m not sure I fully understand what you mean by this.

Right so it seems like when it finds an extra input it does:

1. Flag it
2. Create a warning
3. check that flag only in flush_block, so it will continue to generate inputs until it needs to flush a block again

Looking into it I think raise StopIteration will more cleanly terminate the generator.

[kordusas]

i will try to look into this a bit more, just raise StopIteration crashes the code. I think this shouldnt stop the python code from running.

[MicahGale]

Hmm yeah. Interesting. I might play around with it a bit as well. This should work. That's how iterators are implemented. You just need a __next__ function that either gives the next item, or raise StopIterator and then the for loop should handle it. I wonder if something is causing the StopIterator to raise multiple times? read_input_file does use yield from and I'm not sure how these things interact.

[MicahGale]

Ok I understand the bug fix now, and am happy with it.

We don't need to add extra tests for that bug, because your previous commits showed that this new warning is very effective at finding this bug.

[MicahGale]

I think you will fix #703. Could you try to replicate this bug on your branch and see if you fixed it, and update the comments appropriately?