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

Support cube files without atoms #526

Merged
merged 2 commits into from
Feb 23, 2022
Merged

Support cube files without atoms #526

merged 2 commits into from
Feb 23, 2022

Conversation

pmrv
Copy link
Contributor

@pmrv pmrv commented Feb 7, 2022

Fixes #525.

I've cross-checked with this definition of cube files, is that correct @fldei101, @sudarsan-surendralal what code are we targeting here?

I haven't tested this, because I have no example files, but it should be ok. One issue might be that this will leave self._atoms undefined which could break assumptions of downstream code.

@pmrv pmrv added the bug Something isn't working label Feb 7, 2022
@coveralls
Copy link

coveralls commented Feb 7, 2022

Pull Request Test Coverage Report for Build 1806690681

  • 7 of 7 (100.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.004%) to 70.202%

Files with Coverage Reduction New Missed Lines %
pyiron_atomistics/lammps/structure.py 1 75.08%
Totals Coverage Status
Change from base Build 1802790486: -0.004%
Covered Lines: 11671
Relevant Lines: 16625

💛 - Coveralls

@niklassiemer
Copy link
Member

I haven't tested this, because I have no example files, but it should be ok. One issue might be that this will leave self._atoms undefined which could break assumptions of downstream code.

Maybe we should warn in this case?

@jan-janssen jan-janssen added the format_black reformat the code using the black standard label Feb 7, 2022
Copy link
Member

@sudarsan-surendralal sudarsan-surendralal left a comment

Choose a reason for hiding this comment

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

LGTM!

@stale
Copy link

stale bot commented Feb 21, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 21, 2022
@jan-janssen
Copy link
Member

@pmrv I guess this can be merged - looks good to me

@jan-janssen jan-janssen removed the format_black reformat the code using the black standard label Feb 22, 2022
@stale stale bot removed the stale label Feb 22, 2022
@pmrv pmrv merged commit 31a8ff5 into master Feb 23, 2022
@delete-merged-branch delete-merged-branch bot deleted the naivecube branch February 23, 2022 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Read CubeFile without atoms
6 participants