Interactive plotter

[kaueltzen]

No description provided.

[JaGeo]

Thank you so much for this! I will check it out now in detail :)

[JaGeo]

This is a very good start :)!

• My first suggestion would be to also extend to description class to also include automatic interative plots:

Similar to
describe.plot_cohps(ylim=[-10, 2], xlim=[-4, 4])

we could add:
describe.plot_interactive_cohps(ylim=[-10, 2], xlim=[-4, 4])

• You could maybe start from similar code to (however, this seems to be very slow at the moment):

ia = InteractiveCohpPlotter() ia.add_all_relevant_cohps(complete_cohp, analyse) fig = ia.get_plot(ylim=[-10, 20]) fig.update_layout(title_text=calculation) fig.show()

• You might also need to check out the labels from the automatic plots as well. I think the plots should fit also to the written description.

• Do you have any idea why the code is currently very slow when executed? I think this would be another point to look into. Maybe, you could solve it by using a similar algorithm to the one in the current automatic plots?

• When everything is finished, we also need to update documentation etc.

[kaueltzen]

Thank you for your suggestions!
I used the same algorithm as in plot_cohps(), they are now of similar speed. It was that slow before because I didn't take the cohps from the analyse object but from an additionally created, unnecessarily large CompleteCohp object.
Will soon add an option for non-summed COHP plots.

[JaGeo]

Great! Thank you! 😅 (Have a nice evening 😉!)

[JaGeo]

@ahtakkatha could you again provide a short sample script for testing, please 😅. It would speed up the review from my side.

[JaGeo]

Yes, it would be great, if you would put the integration options to another branch! Thank you!

[naik-aakash]

Todo

• Add/Update tests
• clean up the code
• Fix linting issues

[JaGeo]

@naik-aakash , I think I have addressed most of the general comments. It would be great if you could work on the ones related to the plotting options.

[JaGeo]

There are still many inconsistencies when it comes to snakecase, unfortunately. This is on me as I simply haven't considered such a convention in the naming of arguments in some of my coding projects ... Changing this now would, however, lead to a lot of broken code (but it is maybe worth fixing it when we update the atomate2 schema the next time).

[naik-aakash]

@JaGeo Thank you for fixing so many suggestions already. 😃

@JaGeo @ajjackson, I will work on the rest review suggestions in the code in next week

[naik-aakash]

Hi @JaGeo and @ajjackson , I think I have manage to address the reviewed parts of code. Would be happy to address incase I missed out something or any more suggestions come up. 😃

[JaGeo]

As discussed in person, @naik-aakash will add an example script on how to use the interactive plotter and add some documentatoin to the tutorial part of the repo. If this is there, I am happy to merge.

[naik-aakash]

Hi @ajjackson , I have one last question. Do you have any suggestions for how we have the functionality to change colors of the plotly figures via cli? Typing out a long list of hex codes is a bit inconvenient for users.

[ajjackson]

A convenient way to do this would be to allow colours to be read from matplotlib styles. It's a bit roundabout, but allows consistency with the other run modes.

The other option would be to create some new config format, or borrow one from another project. Any favourites?