-
Notifications
You must be signed in to change notification settings - Fork 8
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
Fix downloads in notebooks #185
Conversation
I also fixed the naming of various |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved, but two comments:
- I don't think it is worthwhile to actually run the MICADO notebook and update the output. The output has not meaningfully changed, so this will add a megabyte or two to the repository for very little benefit. I think it would be better to just change the cells and commit those. Then test them, but discard those changes. (Also the MICADO notebook is broken.. MICADO notebook 3_scopesim_SCAO_4mas_fv-psf.ipynb is broken #161.)
- These changes highlight another problem about deprecating the list argument, because now the interface is rather ugly in some cases.
Agree. I'm not quite sure why the output would even change at all with the same random seeds. Anyway, I now force-pushed those large but useless "changes" away.
Disagree on that one. In all cases except the "LSS-YSO" notebook, the only resulting change was the square brackets, which in my personal opinion actually reads clearer without those, but that's more a matter of taste. Either way I think plus or minus a square bracket is negligible. In the case of the "LSS-YSO" notebook (which I assume was the one you were referring to anyway), the change resulting from the list -> *args modification was from sim.download_example_data(list(fitsfiles.values())) to sim.download_example_data(*fitsfiles.values()) Where the original The remaining "wrap" of fitsfiles = dict(zip(fitsfiles, sim.download_example_data(*fitsfiles.values()))) is some "iterable magic" to get the locations of the cached files back into the |
Good, thank you for fixing yet another one of these! |
Following AstarVienna/ScopeSim#446.
For the PSF download in MICADO, we might eventually expand the download facilities in ScopeSim, once
pooch
has proven to be (more) stable. For now, I used pooch's "script mode" to move AstarVienna/ScopeSim#332 forward.