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.

Sign up for GitHub

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

Jump to bottom

Fixed SyntaxNode AttributeError. #582

Merged
merged 14 commits into from
Oct 30, 2024
Merged

Fixed SyntaxNode AttributeError. #582

merged 14 commits into from
Oct 30, 2024

Conversation

MicahGale
Copy link
Collaborator

@MicahGale MicahGale commented Oct 23, 2024 •
edited
Loading

Description

This is a fix for an AttributeError when adding a leading comment to an Importance that invokes SyntaxNode._grab_beginning_comment which previously was only implemented for PaddingNode. This implements it for SyntaxNodeBase and SyntaxNode which should cover all other use cases.

This also adds a final check to ensure no extra blank lines are being added.

Fixes #580

Checklist

  • I have performed a self-review of my own code
  • 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. labels Oct 23, 2024
@MicahGale MicahGale self-assigned this Oct 23, 2024
@MicahGale MicahGale linked an issue Oct 23, 2024 that may be closed by this pull request
'SyntaxNode' object has no attribute '_grab_beginning_comment' #580
Closed
tjlaboss
tjlaboss reviewed Oct 29, 2024
View reviewed changes
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.

Just tested this branch and it does fix the issue.

@MicahGale MicahGale marked this pull request as ready for review October 29, 2024 23:48
tjlaboss
tjlaboss requested changes Oct 30, 2024
View reviewed changes
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.

There's still one new test failure, but it's almost there.

tests/inputs/test_importance.imcnp Show resolved Hide resolved
tests/test_integration.py Show resolved Hide resolved
tests/test_integration.py Show resolved Hide resolved
doc/source/changelog.rst Show resolved Hide resolved
@MicahGale
Copy link
Collaborator Author

@tjlaboss you are suddenly very motivated to get this merged :).

tjlaboss
tjlaboss approved these changes Oct 30, 2024
View reviewed changes
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.

Two small improvements

montepy/data_inputs/cell_modifier.py Outdated Show resolved Hide resolved
montepy/mcnp_object.py Outdated Show resolved Hide resolved
@MicahGale MicahGale merged commit c0bbeba into develop Oct 30, 2024
20 checks passed
@MicahGale MicahGale deleted the 580-syntaxnode-object-has-no-attribute-_grab_beginning_comment branch October 30, 2024 14:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tjlaboss tjlaboss tjlaboss approved these changes

Assignees

@MicahGale MicahGale

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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

'SyntaxNode' object has no attribute '_grab_beginning_comment'
2 participants
@MicahGale @tjlaboss