-
Notifications
You must be signed in to change notification settings - Fork 59
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
Add MigrationGraph
Renderable
#267
base: main
Are you sure you want to change the base?
Conversation
Hi @hmlli, let me know if there's been any progress on this or if you'd like to meet to discuss? |
@mkhorton sure! I'm still trying to figure out the scheme but I would like to meet to talk about it. I'll ping you in slack |
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
This pull request introduces 1 alert when merging 7e251c4 into b56f413 - view on LGTM.com new alerts:
|
MigrationGraph
ComponentMigrationGraph
Renderable
aab48eb
to
adb5238
Compare
@mkhorton Hi Matt! I think this PR is ready as a first iteration for the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not Matt here but left a few quick comments to save @mkhorton some time.
crystal_toolkit.egg-info/PKG-INFO
Outdated
@@ -0,0 +1,18 @@ | |||
Metadata-Version: 2.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The files in crystal_toolkit.egg-info/*
should be .gitignored
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with dist/crystal_toolkit-2021.4.29-py3.9.egg
and anything else in dist/*
.
app = dash.Dash(assets_folder=SETTINGS.ASSETS_PATH) | ||
|
||
# create the MigrationGraph object | ||
mg = loadfn("LiMnP2O7_mg.json") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried to run the example from the root dir with
python crystal_toolkit/apps/examples/migrationgraph.py
and got the error
FileNotFoundError: [Errno 2] No such file or directory: 'LiMnP2O7_mg.json'
To make the example robust w.r.t. cwd, you might want to change
- mg = loadfn("LiMnP2O7_mg.json")
+ import os
+ module_dir = os.path.dirname(os.path.abspath(__file__))
+ mg = loadfn(f"{module_dir}/LiMnP2O7_mg.json")
@@ -0,0 +1,94 @@ | |||
import json |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Files in build/*
should also be ignored, not committed. Might be worth purging these files from git
history since they're quite large.
Thanks both, reviewing now :) |
MigrationGraph
RenderableMigrationGraph
Renderable
Removed the [WIP] from this PR title, forgot this is in the review queue! Apologies for letting it sit. |
This PR is to address issue #254 .
TO DO: