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

Refactor interface_reactions module, add support for Plotly #2233

Merged
merged 9 commits into from Sep 8, 2021

Conversation

mattmcdermott
Copy link
Member

@mattmcdermott mattmcdermott commented Sep 2, 2021

Summary

  • Add: support for Plotly: calling plot() now provides a backend option for choosing matplotlib or Plotly.
  • Add:GrandPotentialInterfacialReactivity class to separate code based on the use of grand potential phase diagrams (this greatly decreases the complexity of the __init__ args) and is a more inherited approach in general
  • Add: get_dataframe() method for plotting possible reactions in a Pandas dataframe. Users would typically have to write/copy boilerplate code to do this each time
  • Fix: Better matplotlib plotting, included annotations
  • Fix: Cleaner, more readable code

Checklist

  • Code is in the standard Python style. The easiest way to handle this
    is to run the following in the correct sequence on your local machine. Start with running
    black on your new code. This will automatically reformat
    your code to PEP8 conventions and removes most issues. Then run
    pycodestyle, followed by
    flake8.
  • Docstrings have been added in the Google docstring format.
    Run pydocstyle on your code.
  • Type annotations are highly encouraged. Run mypy to type check your code.
  • Tests have been added for any new functionality or bug fixes.
  • All linting and tests pass.

@mattmcdermott mattmcdermott marked this pull request as draft September 2, 2021 22:16
@mattmcdermott
Copy link
Member Author

@mkhorton My plan is to have this PR done by Wed, September 8, based on the previous timeline goal we set

@mkhorton
Copy link
Member

mkhorton commented Sep 2, 2021

Fantastic, thank you for the update @mattmcdermott !

@mattmcdermott mattmcdermott marked this pull request as ready for review September 7, 2021 21:02
@mattmcdermott mattmcdermott changed the title [WIP] Refactor interface_reactions module, add support for Plotly Refactor interface_reactions module, add support for Plotly Sep 7, 2021
@mattmcdermott
Copy link
Member Author

mattmcdermott commented Sep 7, 2021

@mkhorton Should be pretty much done. Seems like linting and tests are failing for recent changes related to Optimade.

Note: you'll see I changed something with PDPlotter -- it was an old method I had written called htmlize_formula(). Turns out that method already existed in pymatgen.util.string, so I removed it from PDPlotter and replaced where it was used.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.6%) to 83.153% when pulling 8492558 on mattmcdermott:interface_rxns into 2c6ac61 on materialsproject:master.

@mkhorton
Copy link
Member

mkhorton commented Sep 8, 2021

Thanks @mattmcdermott! I think that method is also available on Composition etc. directly (via the Stringify mixin), just an fyi.

@mattmcdermott
Copy link
Member Author

mattmcdermott commented Sep 8, 2021

Thanks @mattmcdermott! I think that method is also available on Composition etc. directly (via the Stringify mixin), just an fyi.

Ah true. The only downside is for compositions like Composition("FeO2"), it automatically puts "1" as a subscript:
e.g.,Fe_1 O_2

@mkhorton
Copy link
Member

mkhorton commented Sep 8, 2021

Ah, I guess it acts on the Composition directly rather than formula, fair enough.

@mkhorton mkhorton merged commit 4145ffd into materialsproject:master Sep 8, 2021
@mkhorton
Copy link
Member

mkhorton commented Sep 8, 2021

Merged. Thanks again @mattmcdermott, this is great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants