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

Make atom lines configurable in the input writer #253

Merged
merged 4 commits into from
Jun 5, 2024

Conversation

tovrstra
Copy link
Member

@tovrstra tovrstra commented Apr 1, 2021

Fixes #192.

Few other things are also fixed in this PR:

  • Catch warning in test_input_orca_from_molden
  • Remove unused kwdocs from document_write_input
  • More code reuse between input writers (write_input_base instead of populate_fields)
  • Conversion of atomic coordinates to angstroms is moved to program-specific code. The main motivation is to postpone such conversions as long as possible.

I considered a few ways to implement this, e.g. to pass in atom_line as a template string. Unfortunately, the .format method is not as versatile as the f prefix for string literals, which makes it impossible to get the same functionality with atom_line string templates. Hence, atom_line is a callable.

Few other things are also fixed in this PR:
- Catch warning in test_input_orca_from_molden
- Remove unused kwdocs from document_write_input
- More code reuse between input writers (write_input_base instead of poulate_fields)
- Conversion of atomic coordinates to angstroms is moved to program-specific code.
  The main motivation is to postpone such conversions as long as possible.
@codecov
Copy link

codecov bot commented Apr 1, 2021

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@8712d41). Learn more about missing BASE report.

Current head 66dd4bd differs from pull request most recent head d3b7a5f

Please upload reports for the commit d3b7a5f to get more accurate results.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #253   +/-   ##
=======================================
  Coverage        ?   95.64%           
=======================================
  Files           ?       74           
  Lines           ?     8268           
  Branches        ?     1072           
=======================================
  Hits            ?     7908           
  Misses          ?      165           
  Partials        ?      195           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@FarnazH FarnazH left a comment

Choose a reason for hiding this comment

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

@tovrstra this is a big improvement. Thanks for the clear explanation.

  1. I think I am missing sth, but what is the purpose of _field (dictionary with fields) argument of atom_line? It is not used in the examples given, and I cannot think of how it would be used.

iodata/api.py Outdated Show resolved Hide resolved
iodata/docstrings.py Outdated Show resolved Hide resolved
iodata/inputs/gaussian.py Outdated Show resolved Hide resolved
iodata/inputs/orca.py Outdated Show resolved Hide resolved
@tovrstra
Copy link
Member Author

@FarnazH All should be fine. The failing test on (ubuntu-latest, 3.9) is a caching issue, which is fixed in the master branch.

@tovrstra tovrstra requested a review from FarnazH April 14, 2021 11:07
Copy link

sonarcloud bot commented Jun 3, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

deepsource-io bot commented Jun 5, 2024

Here's the code health analysis summary for commits 8712d41..d3b7a5f. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource Shell LogoShell✅ SuccessView Check ↗
DeepSource Python LogoPython✅ SuccessView Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

@tovrstra
Copy link
Member Author

tovrstra commented Jun 5, 2024

Given that this zomie-PR was already reviewed (long time ago) and all comments were addressed, I'm going to merge it. All tests and checks pass after merging the latest main branch back into this one. A few typos were fixed while preparing this merge.

@tovrstra tovrstra merged commit 7c221cb into theochem:main Jun 5, 2024
10 checks passed
@tovrstra tovrstra deleted the atom-line-template branch June 5, 2024 09:15
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.

Configurable atom lines in write_input
2 participants