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 boolean to retain molecule name #557

Merged
merged 5 commits into from
Jun 18, 2024
Merged

Added boolean to retain molecule name #557

merged 5 commits into from
Jun 18, 2024

Conversation

wverastegui
Copy link
Contributor

This PR adds a Boolean option to retain the original molecule name in the optimized XYZ file if set to "YES". By default, it is set to "NO", preserving the information added by xTB.

closes #534

@wverastegui wverastegui requested a review from hechth June 11, 2024 13:20
Copy link
Member

@hechth hechth left a comment

Choose a reason for hiding this comment

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

The new tests look good, except for the one checking for an empty string - that one should be improved. What was wrong with the previous test for this case?

<assert_contents>
<has_text text="$coord"/>
<has_n_lines min="26"/>
<has_text text=" "/>
Copy link
Member

Choose a reason for hiding this comment

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

This merely tests if it has a space. There is I think an assert to check a the content of a specific line - this could be useful here.

Copy link
Member

Choose a reason for hiding this comment

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

I think the previous check was maybe even more concise in this place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think <not_has_text text="energy"/> fits to this case as well b4eb0e6.

@hechth hechth merged commit 070a3fd into master Jun 18, 2024
10 checks passed
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.

Retain molecule name in optimized XYZ file
2 participants