Improve example notebooks

[teutoburg]

No description provided.

[codecov]

Codecov Report

Patch coverage has no change and project coverage change: -0.43 ⚠️

Comparison is base (62b5ac3) 75.57% compared to head (e70b192) 75.15%.

Additional details and impacted files

@@              Coverage Diff               @@
##           dev_master     AstarVienna/ScopeSim#239      +/-   ##
==============================================
- Coverage       75.57%   75.15%   -0.43%     
==============================================
  Files             152      152              
  Lines           15523    15668     +145     
==============================================
+ Hits            11732    11775      +43     
- Misses           3791     3893     +102     

see 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[hugobuddel]

(I can't seem to comment on the changes themselves, probably because they are notebooks. I also cannot figure out how to 'start a review' without being able to comment on the files, so separate comments it is.)

If I understand correctly, the notebooks are also (or primarily) used in the online documentation (hence them being in the docs/source directory). See for example https://scopesim.readthedocs.io/en/latest/5_liners/bang_strings.html

So I believe the idea of the .. note and .. info blocks is that they are rst directives. However, I don't see them at all in the documentation, so your changes would make things better. Nevertheless, I wanted to ensure you knew about why these .. things are there in the first place.

[hugobuddel]

My American PhD-supervisor taught me that "so-called" is often (usually?) used in a derogatory way. "My so-called friend let me stand in the rain for 3 hours."

I don't know what better language to use though, so it is probably fine in this context.

[hugobuddel]

About storing the output: as the notebooks double as documentation, it might be good to have the output in there.

I believe that read the docs will run the notebooks and thereby creating proper online documentation. If that is indeed the case, then removing them is okay with me.

Maybe we could make the output reproducible by always using a random number seed. That way changing the text would not create new figures, and thereby prevent bloating the repository. But we don't have that yet, so maybe it is good to not keep the figures now.

Note that the figures are now already in the git-history of the branch. So if saving space is your concern, then we should squash this branch, or rebase it interactively.

[teutoburg]

My American PhD-supervisor taught me that "so-called" is often (usually?) used in a derogatory way. "My so-called friend let me stand in the rain for 3 hours."

I don't know what better language to use though, so it is probably fine in this context.

This might be a "lost in translation" issue, or culture dependent. For me, it's more of an explanatory phrase, when introducing new topic-specific terminology. A bit like spoken quotation marks if that makes sense. I'm perfectly aware that in can also be used in a negative, ironic way, depending on the context. This also exists in German btw. Personally I think the context here is clear, but I'm perfectly fine with rephrasing this, if you have a better suggestion, by all means!

[hugobuddel]

Looking good, awesome @teutoburg 🤹 !

The idea behind the TL;DR stuff is easier to understand now.

Maybe we need a more permanent solution to this hackish "required for read the docs" stuff, but for now your changes make it easier to comprehend what is going on for new people.

Some comments above, that you probably already saw, but no show stoppers here. I'm trusting you and the CI that the notebooks still run as intended 🎢

[teutoburg]

About storing the output: as the notebooks double as documentation, it might be good to have the output in there.

I believe that read the docs will run the notebooks and thereby creating proper online documentation. If that is indeed the case, then removing them is okay with me.

Maybe we could make the output reproducible by always using a random number seed. That way changing the text would not create new figures, and thereby prevent bloating the repository. But we don't have that yet, so maybe it is good to not keep the figures now.

Note that the figures are now already in the git-history of the branch. So if saving space is your concern, then we should squash this branch, or rebase it interactively.

I removed the output now after a brief discussion with @oczoske just now. I was gonna mention that, but you were faster to comment on it 😅

Anyway, AFAIK the output was previously stored in the repo. But I might be wrong on that. I'll check again, and if it indeed was not, then I'll squash-merge this.

[teutoburg]

So I believe the idea of the .. note and .. info blocks is that they are rst directives. However, I don't see them at all in the documentation, so your changes would make things better. Nevertheless, I wanted to ensure you knew about why these .. things are there in the first place.

Thanks for pointing this out, I was aware of it! However, we do have at least some users who download and run these notebooks locally. In those cases, using the browser-based Jupyter Notebook viewer, these things look a bit weird and do not stand out in any way. That's why I decided to change the formatting to make it more visible.

[hugobuddel]

Anyway, AFAIK the output was previously stored in the repo. But I might be wrong on that. I'll check again, and if it indeed was not, then I'll squash-merge this.

The figures were already there, but if size is your concern, then you should squash, because otherwise the output is still in the git history. That would be the worst case: the figures still take up size in the repository, but they cannot (easily) be seen.

It is only 500kb, so I'm not going to worry about whether you squash or not.

[teutoburg]

About storing the output: as the notebooks double as documentation, it might be good to have the output in there.
I believe that read the docs will run the notebooks and thereby creating proper online documentation. If that is indeed the case, then removing them is okay with me.
Maybe we could make the output reproducible by always using a random number seed. That way changing the text would not create new figures, and thereby prevent bloating the repository. But we don't have that yet, so maybe it is good to not keep the figures now.
Note that the figures are now already in the git-history of the branch. So if saving space is your concern, then we should squash this branch, or rebase it interactively.

I removed the output now after a brief discussion with @oczoske just now. I was gonna mention that, but you were faster to comment on it 😅

Anyway, AFAIK the output was previously stored in the repo. But I might be wrong on that. I'll check again, and if it indeed was not, then I'll squash-merge this.

I just checked and the output was always stored so far, at least for the example notebooks, also back in the master branch. So no recent change either. Anyway, while it doesn't make a huge difference, I think it's still best to store them without output from now on. If we find a good reason to go back to storing with output included, or if this breaks Readthedocs somehow, we can always rollback 🙃

[hugobuddel]

About the coverage, I created #240

I believe we have discussed everything; it seems we can merge this right?