Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Jump to bottom

Cli updates #357

Merged
merged 32 commits into from
Jan 13, 2025
Merged

Cli updates #357

merged 32 commits into from
Jan 13, 2025

Conversation

naik-aakash
Copy link
Collaborator

@naik-aakash naik-aakash commented Jan 1, 2025 •
edited
Loading

Closes #344 and #328

Note: Failing tests in CI are due to the pymatgen depreciation of some methods and have nothing to do with LobsterPy / pymatgen LOBSTER parsers/changes in this PR.

Todo

  • coloured icoxx distance plots via CLI
  • add option to get BWDF plots
  • Update cli docs and tutorials to showcase BWDF / ICOXX distance plotting
  • Option to save raw plot data of relevant (I)COXX as jsons

@naik-aakash naik-aakash self-assigned this Jan 1, 2025
@naik-aakash naik-aakash added the enhancement New feature or request label Jan 1, 2025
@JaGeo
Copy link
Owner

JaGeo commented Jan 3, 2025

Can we fix the LobsterPy tests here first and then add new functionalities? @naik-aakash

@naik-aakash
Copy link
Collaborator Author

naik-aakash commented Jan 3, 2025 •
edited
Loading

Hi @JaGeo , as I have written in the note, it has nothing to do with LobsterPy tests. We have to wait for new release of pymatgen where deprecated method is removed causing failures due to monty warnings

See here : materialsproject/pymatgen#4243

@naik-aakash naik-aakash mentioned this pull request Jan 3, 2025
Closed
@JaGeo
Copy link
Owner

JaGeo commented Jan 3, 2025

@naik-aakash thank you 😅. I also wrote something in the pymatgen PR

@naik-aakash naik-aakash changed the title [WIP] Cli updates Cli updates Jan 5, 2025
@naik-aakash naik-aakash enabled auto-merge January 5, 2025 15:19
@naik-aakash naik-aakash requested review from JaGeo and removed request for JaGeo January 5, 2025 15:19
@naik-aakash naik-aakash requested a review from JaGeo January 6, 2025 08:10
kaueltzen
kaueltzen suggested changes Jan 7, 2025
View reviewed changes
@naik-aakash
Copy link
Collaborator Author

Hi @kaueltzen, I have now addressed the review comments. Let me know if anything else needs to be addressed.

kaueltzen
kaueltzen reviewed Jan 10, 2025
View reviewed changes
Copy link
Contributor

@kaueltzen kaueltzen 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! just some small suggestions

@kaueltzen
Copy link
Contributor

@JaGeo am done, this is ready to be merged imo (: 👍

@naik-aakash naik-aakash requested a review from JaGeo January 13, 2025 17:05
JaGeo
JaGeo reviewed Jan 13, 2025
View reviewed changes
JaGeo
JaGeo reviewed Jan 13, 2025
View reviewed changes
src/lobsterpy/plotting/__init__.py
@@ -1234,3 +1235,128 @@ def get_plot(
ax.scatter(x, y, s=marker_size, marker=marker_style, label=label)

return plt


class BWDFPlotter:
Copy link
Owner

Choose a reason for hiding this comment

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

Could be something for pymatgen in the future once BWDF in Lobster are more established

@naik-aakash naik-aakash merged commit ab90778 into JaGeo:main Jan 13, 2025
22 checks passed
@JaGeo
Copy link
Owner

JaGeo commented Jan 13, 2025

@naik-aakash and @kaueltzen thank you both!

Let me know if a new version is required!

# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@kaueltzen kaueltzen kaueltzen requested changes

@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.

Option to plot BWDF and coloured icoxx distance plots via CLI
3 participants
@naik-aakash @JaGeo @kaueltzen