initial version of MITRE ATT&CK notebook #65
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| @@ -0,0 +1,2063 @@ | |||
| { | |||
There was a problem hiding this comment.
| @@ -0,0 +1,2063 @@ | |||
| { | |||
There was a problem hiding this comment.
This is a monster cell, maybe consider having a bit more details in Markdown about what its doing, and have an output of some sort so its clearer that it has done as expected.
Reply via ReviewNB
There was a problem hiding this comment.
That's the biggest cell I've ever seen!
If it's all just utility functions that are executed and forgotten it's OK but if it's actually doing something procedural, I agree about the comments and/or break it up.
| @@ -0,0 +1,2063 @@ | |||
| { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
The image is very unusual in shape. I tried few percetage options but seems they do not render/display properly.
| @@ -0,0 +1,2063 @@ | |||
| { | |||
There was a problem hiding this comment.
Why do we print out the same data as in the table above (just in a harder to understand format)?
Reply via ReviewNB
There was a problem hiding this comment.
Different format after data reshaping . the output requires for plotly figures to plot
| @@ -0,0 +1,2063 @@ | |||
| { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
Purpose is to show data structure format require to plot the graph. I think verbosity will help to users.
| @@ -0,0 +1,2063 @@ | |||
| { | |||
There was a problem hiding this comment.
There was a problem hiding this comment.
requires jupyterlab-plotly extension. added comments.
|
Overall it works but there are a few usability areas that could do with addressing (you can always do some of them at a later date). |
|
View / edit / reply to this conversation on ReviewNB ianhelle commented on 2022-02-04T19:27:58Z Line #9. from ipywidgets import widgets as ipywidgets, Layout Many of these are imported by init_notebook so don't need them here unless you want to keep them for clarity of what you would need to import. I think that it's reasonable to leave it in - just FYI |
|
View / edit / reply to this conversation on ReviewNB ianhelle commented on 2022-02-04T19:27:59Z Line #27. display(HTML("
Starting Notebook setup...
"))
Can you get rid of the whole section nbcheck? |
|
View / edit / reply to this conversation on ReviewNB ianhelle commented on 2022-02-04T19:28:00Z Line #97. for i in range(0, 5): Don't need to change this but if I was coding this I'd do something more like:
|
No description provided.