Add documentation for Spectral Element Decomposition Plot #1438
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1438 +/- ##
=======================================
Coverage 71.13% 71.13%
=======================================
Files 67 67
Lines 5523 5523
=======================================
Hits 3929 3929
Misses 1594 1594 Continue to review full report at Codecov.
|
jaladh-singhal
force-pushed
the
sedec-doc
branch
from
faca151 to
36d0fb7
Compare
|
View / edit / reply to this conversation on ReviewNB jamesgillanders commented on 2021-02-05T17:18:46Z It would be best to define your abbreviation to SDEC here and then from here on out only use SDEC in the text; e.g. Spectral element DEComposition (SDEC) Plot or similar.
Can you embed the url to the paper cited here?
It is a spectral diagnostic plot similar... - drop the plural on diagnostic
|
|
View / edit / reply to this conversation on ReviewNB jamesgillanders commented on 2021-02-05T17:18:47Z Just use SDEC
... to produce a/an SDEC Plot |
|
View / edit / reply to this conversation on ReviewNB jamesgillanders commented on 2021-02-05T17:18:48Z Can just use SDEC throughout
|
|
View / edit / reply to this conversation on ReviewNB jamesgillanders commented on 2021-02-05T17:18:48Z ... to get a highly...
use SDEC |
|
View / edit / reply to this conversation on ReviewNB jamesgillanders commented on 2021-02-05T17:18:49Z ...SDEC plot is produced from the virtual packet population... |
|
View / edit / reply to this conversation on ReviewNB jamesgillanders commented on 2021-02-05T17:18:50Z the real packet population |
|
View / edit / reply to this conversation on ReviewNB jamesgillanders commented on 2021-02-05T17:18:51Z You can plot in units of flux on the Y-axis of the SDEC plot, by specifying the |
|
View / edit / reply to this conversation on ReviewNB jamesgillanders commented on 2021-02-05T17:18:51Z distance: ...flux instead of luminosity
figsize: (10,7) - this has changed, right? |
|
View / edit / reply to this conversation on ReviewNB jamesgillanders commented on 2021-02-05T17:18:52Z can just use SDEC here
...are specific to the plotting library. We can reproduce all the plots above... |
|
View / edit / reply to this conversation on ReviewNB jamesgillanders commented on 2021-02-05T17:18:53Z *luminosity |
|
View / edit / reply to this conversation on ReviewNB jamesgillanders commented on 2021-02-05T17:18:53Z This |
There was a problem hiding this comment.
This is very nicely presented @jaladh-singhal - very nice notebook! Although I appear to have loads of comments, they are mostly small grammatical corrections to the wording in certain parts. Happy for this to be merged when these have been addressed
|
View / edit / reply to this conversation on ReviewNB jamesgillanders commented on 2021-02-08T12:45:12Z The units on y-axis don't look right - the ^{2} isn't rendering as a superscript 2. Put inside a set of $$ and it should work |
There was a problem hiding this comment.
Corrections look good @jaladh-singhal. I had one final thing I noticed for the y-axis flux label, but apart from that it looks perfect!
@jamesgillanders yes I noticed that but didn't correct that intentionally because #1446 is going to correct it anyway and it's kinda irrelevant to this doc PR. |
Alright, I didn't realise that. Everything else is fine so I will approve now |
|
Looks great! Happy to merge. |
…1438) * Restructured gui index to incoprorte viz tools * Added notebook to demo SEDec plot * Added notebook demonstrating SEDec plot * Renamed SEDec to SDEC * Added distance option & additional options usage * Changed ref link name in index * Added gif to demo notebook * Increase matplotlib plot size * Fixed inconsistency in docstring * Made grammatical edits in notebook & hid warnings * Fixed top level heading from ## to #
Added notebook in gui docs demonstrating use of
sdec_plotmodule. And restructured gui doc index to incorporate both tools and widgets.How Has This Been Tested?
Doc builds correctly
Screenshots (if appropriate):
Types of changes
Checklist: