Skip to content
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

TL;DR #111

Open
4 tasks
hugobuddel opened this issue Jun 21, 2023 · 6 comments
Open
4 tasks

TL;DR #111

hugobuddel opened this issue Jun 21, 2023 · 6 comments
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@hugobuddel
Copy link
Collaborator

hugobuddel commented Jun 21, 2023

Some usability issues that we identified today:

  • We shouldn't use the term TL;DR, and instead explain what we mean. Especially in the notebooks and in the names of the notebooks.
  • The TL;DR at the end of https://github.com/AstarVienna/ScopeSim/blob/dev_master/docs/source/examples/1_scopesim_intro.ipynb should not be a markdown cell but a regular cell so it will be ran in the CI and we get notified if it breaks. This might also apply to other cells.
  • The code in markdown cells should be runnable by copy pasting it in a normal cell. E.g. cell that ends with a line with a colon like class MyClass: should be followed by a line with ellipsis, ....
  • The notebooks contain some code to set the local_package_path for the benefit of Readthedocs. This is confusing and should be rephrased to be useful for the user.

About the last point, the top cell of each notebook currently contains something like

# [Required for Readthedocs] Comment out this line if running locally
tmpdir = TemporaryDirectory()
sim.rc.__config__["!SIM.file.local_packages_path"] = tmpdir.name

Instead it could contain something like

# The code in the next cell will download instrument package from the IRDB.
# If you have already downloaded them before, you can specify the directory
# by setting sim.rc.__config__["!SIM.file.local_packages_path"]
# Here we create a temporary directory and use that, which is useful when
# generating the documentation on readthedocs. 
# The local directory will be used if you don't specify any directory, e.g. by commenting out the next two lines.
tmpdir = TemporaryDirectory()
sim.rc.__config__["!SIM.file.local_packages_path"] = tmpdir.name

Or something similar

@teutoburg
Copy link
Contributor

I think I got all cases of this in the ScopeSim repo. There are however more hidden in some instrument packages in the IRDB repo. I'll transfer the issue and solve the cases there separately...

@teutoburg teutoburg transferred this issue from AstarVienna/ScopeSim Jul 3, 2023
@teutoburg
Copy link
Contributor

There are several notebooks also in this repo where this issue exists. Reopening it here to deal with that...

@teutoburg teutoburg reopened this Jul 3, 2023
@oczoske
Copy link
Collaborator

oczoske commented Jul 3, 2023

I can take care of the local_package_path issue in the METIS notebooks.

A general remark: jupyter notebooks should be saved empty, without the code output (Kernel -> Restart & Clear Output). The resulting file size is smaller and diffs between versions are much more manageable that way, in particular when the output contains plots and images.

@oczoske oczoske self-assigned this Jul 3, 2023
@hugobuddel
Copy link
Collaborator Author

For MICADO I 'solved' the local_package_path simply by adding a symlink called inst_pkgs that points to the root of the repository, and subsequently commented out all the 'download this package' line.

@hugobuddel
Copy link
Collaborator Author

About the figures, it is good to be able to see the figures. That way people know at least what to expect before they run them.

Perhaps we can ensure that the figures are included on read the docs, then people can use that as a reference. But currently the read the docs does not have figures, at least not for these notebooks: https://irdb.readthedocs.io/en/latest/MICADO/docs/example_notebooks/1_scopesim_MCAO_4mas_galaxy.html#

I prefer to keep the figures in the repository as long as they are unavailable on read the docs.

We could also make the notebooks fully reproducible, e.g. by setting the random seeds. Then the figures shouldn't change that often, and then I'd be fine with storing them.

@hugobuddel
Copy link
Collaborator Author

For MICADO I 'solved' the local_package_path simply by adding a symlink called inst_pkgs that points to the root of the repository, and subsequently commented out all the 'download this package' line.

Note that a benefit of the symlink is that running the notebooks will actually use the IRDB from this repository and thus the same version as the notebooks themselves. Otherwise the notebooks will download a package that might, or might not, correspond to the specific commit that the notebooks are from.

@teutoburg teutoburg added the documentation Improvements or additions to documentation label Nov 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: 🆕 New
Development

No branches or pull requests

3 participants