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

Move some of the structure plotting functions from contrib #710

Merged
merged 10 commits into from
Aug 10, 2022
Merged

Conversation

pmrv
Copy link
Contributor

@pmrv pmrv commented Aug 9, 2022

They were previously defined on the TrainingContainer over in contrib, but I have added also distances to plot a plain histogram of nearest neighbor distances.

Corresponding PR to contrib once this is through.

@pmrv pmrv added the enhancement New feature or request label Aug 9, 2022
@pmrv pmrv requested a review from Leimeroth August 9, 2022 09:58
@Leimeroth Leimeroth added the format_black reformat the code using the black standard label Aug 9, 2022
@Leimeroth
Copy link
Member

So TrainingContainers plotting interface should inherit from this than?

@pmrv
Copy link
Contributor Author

pmrv commented Aug 9, 2022

Yeah, that's how I was planning it. These plotting functions would be purely concerned with structural data and the training container ones would then only add things related to energies, forces, etc.

Copy link
Member

@Leimeroth Leimeroth left a comment

Choose a reason for hiding this comment

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

I like the idea. If some errors occur down the line we should fix them there.

@pmrv
Copy link
Contributor Author

pmrv commented Aug 9, 2022

I didn't want to pull in seaborn for just one plot, so I put it in an ImportAlarm. @niklassiemer are we tracking our opt deps somewhere?

@pmrv
Copy link
Contributor Author

pmrv commented Aug 9, 2022

I also remembered to update the change log. :')

@coveralls
Copy link

coveralls commented Aug 9, 2022

Pull Request Test Coverage Report for Build 2829084886

  • 20 of 127 (15.75%) changed or added relevant lines in 2 files are covered.
  • 202 unchanged lines in 2 files lost coverage.
  • Overall coverage decreased (-0.4%) to 68.223%

Changes Missing Coverage Covered Lines Changed/Added Lines %
pyiron_atomistics/atomistics/job/structurecontainer.py 1 6 16.67%
pyiron_atomistics/atomistics/structure/structurestorage.py 19 121 15.7%
Files with Coverage Reduction New Missed Lines %
pyiron_atomistics/atomistics/job/structurecontainer.py 1 74.71%
pyiron_atomistics/vasp/base.py 201 65.51%
Totals Coverage Status
Change from base Build 2819709769: -0.4%
Covered Lines: 11866
Relevant Lines: 17393

💛 - Coveralls

Copy link
Member

@jan-janssen jan-janssen left a comment

Choose a reason for hiding this comment

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

Looks good to me

@jan-janssen jan-janssen added format_black reformat the code using the black standard and removed format_black reformat the code using the black standard labels Aug 9, 2022
@pmrv pmrv merged commit f1b91a5 into master Aug 10, 2022
@delete-merged-branch delete-merged-branch bot deleted the splot branch August 10, 2022 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request format_black reformat the code using the black standard
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants