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

Fixed Parser bug with Source Definition #431

Merged
merged 32 commits into from
Jul 17, 2024

Conversation

MicahGale
Copy link
Collaborator

@MicahGale MicahGale commented Jul 2, 2024

Description

This was a parser bug fix that ultimately led to some bigger infrastructure upgrades.

So #396 was silent because it was able to log errors with parsing, recover, and then parse something slightly meaningful. Changes made to fix this:

  1. Check the error log with every parsing, and return None if there are any errors. This triggers higher up error handling.
  2. Eliminated the need for stub montepy.data_input classes for switching out parsers. Instead implemented _load_correct_parser which updates self._parser based on the Input classifier.
  3. sdef has a lot of d1 etc which is interpreted as a particle and a number in a ListNode. Modified append to never_pad d on formatting based on there being no trailing space.
  4. Added flag to ListNode.append to only add never_pad during the parsing step
  5. Made ListNode also respect never_pad
  6. Changed logic around Jumps not being never_pad
  7. Fixed the parser by making a hacking DataParser subclass that parses inputs that are only key-value pairs.
  8. Added all sdef keywords to the tokens to recognize them.

Fixes #396, Fixes #427.

Checklist

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation (if applicable)
  • I have added tests that prove my fix is effective or that my feature works (if applicable)

@MicahGale MicahGale added bugs A deviation from expected behavior that does not reach the level of being reportable as an "Error". critical An issue that seriously limits user adoption or hampers current use. parsers are hard Examples of where MCNP syntax is complicated and should be simplified. labels Jul 2, 2024
@MicahGale MicahGale self-assigned this Jul 2, 2024
@MicahGale MicahGale linked an issue Jul 2, 2024 that may be closed by this pull request
@MicahGale MicahGale changed the title Fixed Parser bug with Fixed Parser bug with Source Definition Jul 2, 2024
@MicahGale
Copy link
Collaborator Author

MicahGale commented Jul 2, 2024

@MicahGale MicahGale requested a review from tjlaboss July 10, 2024 13:52
@MicahGale MicahGale marked this pull request as ready for review July 10, 2024 14:16
@MicahGale MicahGale enabled auto-merge July 16, 2024 02:29
@MicahGale MicahGale mentioned this pull request Jul 17, 2024
Copy link
Collaborator

@tjlaboss tjlaboss left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good unless I've misunderstood anything in the comments I've left.

tests/test_source.py Show resolved Hide resolved
tests/test_integration.py Show resolved Hide resolved
montepy/input_parser/tokens.py Show resolved Hide resolved
montepy/data_inputs/data_input.py Outdated Show resolved Hide resolved
montepy/input_parser/data_parser.py Outdated Show resolved Hide resolved
montepy/input_parser/tally_parser.py Show resolved Hide resolved
montepy/input_parser/tally_seg_parser.py Show resolved Hide resolved
montepy/input_parser/data_parser.py Show resolved Hide resolved
@MicahGale MicahGale disabled auto-merge July 17, 2024 21:57
@MicahGale MicahGale enabled auto-merge July 17, 2024 21:58
@MicahGale MicahGale merged commit 6df9d88 into develop Jul 17, 2024
14 checks passed
@MicahGale MicahGale deleted the 396-silent-parsing-error-with-sdef branch July 17, 2024 21:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugs A deviation from expected behavior that does not reach the level of being reportable as an "Error". critical An issue that seriously limits user adoption or hampers current use. parsers are hard Examples of where MCNP syntax is complicated and should be simplified.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add finalize GH release as part of the deploy checklist Silent parsing error with sdef
2 participants