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

COBI COOP extension #128

Merged
merged 17 commits into from
Sep 14, 2023
Merged

COBI COOP extension #128

merged 17 commits into from
Sep 14, 2023

Conversation

naik-aakash
Copy link
Collaborator

@naik-aakash naik-aakash commented Jul 28, 2023 •
edited
Loading

This PR links to #125

Todos

  • Add tests for analyze and describe
  • Fix bugs
  • Update documentation
  • Update describe module and automatic plots
  • Update Interactiveplotter to work for cobi and coop
  • update cli
  • add cli tests for coop/cobi

@naik-aakash naik-aakash changed the base branch from cobi_coop_extension to main July 31, 2023 11:42
@naik-aakash
Copy link
Collaborator Author

naik-aakash commented Jul 31, 2023 •
edited
Loading

Hi @JaGeo, I will hold on to add tests until #102 is merged. I could then reuse test files from there.

Currently, the existing tests are failing as it needs a new release of pymatgen with coop and cobi functionality implemented recently > Update to LobsterNeighbors to work for coop/cobi

@JaGeo
Copy link
Owner

JaGeo commented Jul 31, 2023

@naik-aakash you could ask for such a release already

@naik-aakash naik-aakash self-assigned this Aug 17, 2023
@naik-aakash naik-aakash added the enhancement New feature or request label Aug 17, 2023
@naik-aakash naik-aakash changed the title [WIP] Cobi coop extension Cobi coop extension Aug 18, 2023
@naik-aakash naik-aakash changed the title Cobi coop extension COBI COOP extension Aug 18, 2023
@naik-aakash
Copy link
Collaborator Author

This PR is ready for review. 😃

@naik-aakash
Copy link
Collaborator Author

Should also Close #21 ?

JaGeo
JaGeo reviewed Sep 6, 2023
View reviewed changes
lobsterpy/plotting/test/test_plotting.py
@@ -13,7 +14,7 @@
TestDir = CurrentDir / "../../"


class InteractiveCohpPlotterTest(unittest.TestCase):
class PlainInteractiveCohpPlotterTest(unittest.TestCase):
Copy link
Owner

Choose a reason for hiding this comment

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

Is this okay? Not sure why it is called Plain now?

@JaGeo
Copy link
Owner

JaGeo commented Sep 6, 2023

Could you also resolve the conflicts, @naik-aakash ? Thanks!

@naik-aakash
Copy link
Collaborator Author

Could you also resolve the conflicts, @naik-aakash ? Thanks!

Done

JaGeo
JaGeo approved these changes Sep 13, 2023
View reviewed changes
Copy link
Owner

@JaGeo JaGeo left a comment

Choose a reason for hiding this comment

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

I have tested the obvious cases. Text, automatic plot and interactive automatic plot reflect the change to COBI, COOP

@JaGeo
Copy link
Owner

JaGeo commented Sep 13, 2023

once the merge conflicts are solved, I can merge.

@naik-aakash
Copy link
Collaborator Author

Thanks @JaGeo , will resolve them as soon as possible today

@naik-aakash
Copy link
Collaborator Author

Hi @JaGeo , issues are resolved so this could be merged 😄

@JaGeo JaGeo merged commit 990082f into JaGeo:main Sep 14, 2023
21 checks passed
@JaGeo JaGeo mentioned this pull request Sep 14, 2023
Closed
@naik-aakash naik-aakash deleted the cobi_coop_extension branch June 20, 2024 06:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JaGeo JaGeo JaGeo approved these changes

Assignees

@naik-aakash naik-aakash

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@naik-aakash @JaGeo