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

Added script to fix ignored keywords #148

Closed
wants to merge 4 commits into from

Conversation

hakonhagland
Copy link
Collaborator

See #52 for background.

Copy link
Member

@blattms blattms left a comment

Choose a reason for hiding this comment

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

This looks really cool. Thanks a lot.

@lisajulia can you have a look at the code and merge, please?

Comment on lines +37 to +46
if match := re.match(r"""(\s*<text:p\s+.*?>)(.*?)(</text:p>.*$)""", line, re.DOTALL):
start, txt, end = match.groups()
# Remove span tags from txt
txt = re.sub(r"""<text:span .*?>(.*?)</text:span>""", r"\1", txt)
txt = re.sub(r"""<text:span>(.*?)</text:span>""", r"\1", txt)
if "is ignored by OPM Flow and has no effect on the simulation" in txt:
logging.info(f"Found ignored match in {file}:{line_no}.")
txt = self.replace_ignored(txt, critical)
line = start + txt + end
changed = True
Copy link
Member

Choose a reason for hiding this comment

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

Just one question. There is ugly case with ... simulatio</text:span><text:span text:style-name="T2789">n.</text:span>. That is probably not covered and needs to be done manually. I guess that is ok, though as we cannot anticipate these kind of formatting things.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That is probably not covered and needs to be done manually.

@blattms If you mean the case that there is no space between closing and starting tag: </text:span><text:span>, it should also work. I tested it now.

@click.command()
@ClickOptions.maindir(required=False)
def fix_ignored(maindir: str
) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the line break here intentional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No they are not :) Thanks, for the comment, I will fix this. (I have problems pushing to this branch for some reason, so need to fix this problem first)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have problems pushing to this branch for some reason, so need to fix this problem first

I have updated the PR in a new PR, see #149.

@hakonhagland
Copy link
Collaborator Author

hakonhagland commented Feb 9, 2024

Replaced by #150.

lisajulia added a commit that referenced this pull request Feb 9, 2024
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

Successfully merging this pull request may close these issues.

3 participants