Add a button to download bandstructure and DOS Json data (#46) #47
Conversation
|
@unkcpz could you help me to check why the "pre-commit" CI failed? |
|
Changing |
There was a problem hiding this comment.
@dou-du thanks, I go to https://hub-binder.mybinder.ovh/user/osscar-org-widget-bandsplot-uumbakpe/doc/tree/examples/bandsplot-example.ipynb but can not find the button and to check it.
.github/workflows/ci.yml
Outdated
| @@ -19,7 +19,7 @@ jobs: | |||
|
|
|||
| - name: Install dependencies | |||
| run: | | |||
| python -m pip install pre-commit==2.11.1 | |||
| python -m pip install pre-commit==3.0.3 | |||
There was a problem hiding this comment.
This need to be compatible with the pre-commit.yaml, I guess you change the to fix the pre-commit issue. If it is not work, please revert it back to old version.
| theDownloadJsonButton.onclick = function () { | ||
| if ($.isEmptyObject(dosFile)) { | ||
| var a = document.createElement('a'); | ||
| if (dataFilePaths.length) { |
There was a problem hiding this comment.
I don't understand this for loop, it will go through every band and download or it will download all as a archived file?
There was a problem hiding this comment.
It will download mutiple json files, if more than one bandstructure data were given.
| theDownloadJsonButton.onclick = function () { | ||
| var a = document.createElement('a'); | ||
|
|
||
| if (dataFilePaths.length) { |
There was a problem hiding this comment.
I think the length is to check if the data exist. So rather than check the length it would be clear to check the existence directly.
There was a problem hiding this comment.
dataFilePaths can be empty, if only DOS file was given.
There was a problem hiding this comment.
Okay, I see, but the for dos and bands data here the variable's name are messed up and very confusion. If you can fix it in another PR, that would be helpful for future maintenance. Can you do that?
There was a problem hiding this comment.
I agree this vriable is confusion. I am going to change it.
Please check this binder link: |
As I said, this should fix the pre-commit error. |
It is super slow to access this URL, is there other way I can see the effect and test? Is there any reason that cause the notebook load so slow, can we fix that? |
| theDownloadJsonButton.onclick = function () { | ||
| var a = document.createElement('a'); | ||
|
|
||
| if (dataFilePaths.length) { |
There was a problem hiding this comment.
Okay, I see, but the for dos and bands data here the variable's name are messed up and very confusion. If you can fix it in another PR, that would be helpful for future maintenance. Can you do that?
|
Please check the fix aiidalab/aiidalab-widgets-base#428 |
Add a button to download bandstructure and DOS Json data together.