-
Notifications
You must be signed in to change notification settings - Fork 4
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
Firefly version of DM stack tutorial #4
base: answers
Are you sure you want to change the base?
Conversation
84cda37
to
0001c36
Compare
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.
Here are a few comments:
- I would prefer if we were consistent in the environment where we expect these to run. So far, that has been the JupyterLab environment. Unless there is a driving reason, I think we should stay with that as the default.
- Depending on what happens with 1), the commented out code needs to be removed. It's confusing to have two essentially identical lines with one commented out and the other not.
- I think you could make this notebook more firefly centric. Since the main purpose is to show how to use firefly as a vis engine, I think you could take out all the matplotlib visualizations and just show how to work with firefly. You could probably even pare things down a little more to showcase the visualization.
- There are some places where the notebook calls low level client methods. That breaks our display abstraction pretty fundamentally. I think those need to be wrapped up in the display abstract classes.
- I think it would be good if the channel name was safe even if several people are running the same notebook. It wouldn't take much to make a channel name with a random string. This has some nice one liners.
- When you spin up the server, you can print a link to click on with something like this:
from IPython.display import display, Markdown
display(Markdown('Open a browser window to [here](http://{}/firefly/slate.html?__wsch={})'.format(my_server, my_channel)))
, but watch out because that imports a method that is shadowed by a method definition in this notebook.
7. In the blended example, it would be nice if all the frames were the same image scaling so it's easier to compare.
8. You can probably take out the Working with Catalogs section since it has no visualization aspect, though I guess that's where the catalog comes from that you plot in the next section.
9. In the exercises, I think it would be fine to just keep the ones that show some firefly aspects.
10. In the PSF grid, is it necessary to have a 4x4 grid with a column of zero images? It would look better if only the cells with data were plotted.
I can no longer push to this repository. New pull request is at lsst-dm#2 |
Starting with the tutorial notebook on the answers branch, commands for image display and plotting using Firefly have been added, alongside the original matplotlib display commands.
A short README_firefly Markdown file has also been added.