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.

Sign up for GitHub

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

Jump to bottom

Fix BandsData issue related to the k-points labels #2492

Merged
merged 3 commits into from
Feb 28, 2019
Merged

Fix BandsData issue related to the k-points labels #2492

merged 3 commits into from
Feb 28, 2019

Conversation

yakutovicha
Copy link
Contributor

fixes #2491

giovannipizzi
giovannipizzi reviewed Feb 15, 2019
View reviewed changes
Copy link
Member

@giovannipizzi giovannipizzi left a comment

Choose a reason for hiding this comment

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

Hi @yakutovicha, do you know if it's easy to fix also the other problem (namely, if the first point with a label is not the one with index = 0, how to avoid that the first part of the band is not shown, and similarly for the last point in the path?)

@giovannipizzi
Copy link
Member

Sorry, I see that it's in the issue.
Maybe then better to put a [WIP] on this PR until all points in the issue are fixed?

@coveralls
Copy link

coveralls commented Feb 15, 2019 •
edited
Loading

Coverage Status

Coverage increased (+1.1%) to 56.066% when pulling bc56dd6 on yakutovicha:fix_2491_bandsdata_kpoint_labels into 28eea8c on aiidateam:release_v0.12.3.

@yakutovicha yakutovicha changed the title Fix BandsData issue related to the k-points labels [WIP] Fix BandsData issue related to the k-points labels Feb 16, 2019
@yakutovicha
Copy link
Contributor Author

Maybe then better to put a [WIP] on this PR until all points in the issue are fixed?

Hi Giovani, thanks, yes I forgot to put [WIP] yesterday, now it is there.

@yakutovicha yakutovicha mentioned this pull request Feb 25, 2019
Merged
2 tasks
@yakutovicha
fix problem with empty labels array
f0a2e9e
@yakutovicha
Fix bands plotting
83df91d 
If the first and/or the last kpoint label does not point to the first and/or
the last kpoint - the plotted band structure was cut on the left and/or right
sides. To avoid this I add empty labels to the first and/or the last kpoints
if they are not labeled.
@yakutovicha
Add documentation on how to use the BandsData object
bc56dd6
@yakutovicha
Copy link
Contributor Author

@giovannipizzi, I think I am done with the PR. Please review it and let me know if something else still needs to be done.

@yakutovicha yakutovicha changed the title [WIP] Fix BandsData issue related to the k-points labels Fix BandsData issue related to the k-points labels Feb 28, 2019
@ltalirz
Copy link
Member

ltalirz commented Feb 28, 2019

There currently is an issue with matplotlib on macos, so I can't really test it:

 bs.show_mpl() # to visualize the bands
   ...:
2019-02-28 15:06:39.727 python[60328:12630184] -[TKWindow setCanCycle:]: unrecognized selector sent to instance 0x112bd6430
2019-02-28 15:06:39.733 python[60328:12630184] *** Terminating app due to uncaught exception 'NSInvalidArgumentException', reason: '-[TKWindow setCanCycle:]: unrecognized selector sent to instance 0x112bd6430'

I'll test on QM when I have time

ltalirz
ltalirz approved these changes Feb 28, 2019
View reviewed changes
Copy link
Member

@ltalirz ltalirz left a comment

Choose a reason for hiding this comment

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

tested on ubuntu & works. thanks for the fix!

@ltalirz ltalirz merged commit f757e55 into aiidateam:release_v0.12.3 Feb 28, 2019
giovannipizzi
giovannipizzi reviewed Feb 28, 2019
View reviewed changes
docs/source/datatypes/bands.rst

labels = [(0, u'GAMMA'),
(5, u'X'),
(6, u'X'),
Copy link
Member

Choose a reason for hiding this comment

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

Thanks! Just a question, is this meant to be X or should it be Y?

@giovannipizzi
Copy link
Member

Also, could you also do a PR against provenance_redesign? Thanks!

@ltalirz ltalirz added this to PRs to be cherry-picked in Port from 0.12.3 into develop Mar 1, 2019
@sphuber sphuber mentioned this pull request Mar 6, 2019
Closed
3 tasks
@yakutovicha yakutovicha deleted the fix_2491_bandsdata_kpoint_labels branch January 6, 2020 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@giovannipizzi giovannipizzi giovannipizzi left review comments

@ltalirz ltalirz ltalirz approved these changes

Assignees
No one assigned
Labels
None yet
Projects
No open projects
Port from 0.12.3 into develop
  
PRs to be cherry-picked
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@yakutovicha @giovannipizzi @coveralls @ltalirz