-
Notifications
You must be signed in to change notification settings - Fork 49
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
Add Auth for SPARCL section #248
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
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.
Hi @jacquesalice, thanks for your work on this! Overall it looks really good and I see that you made a few fixes besides Auth. I tried writing my comments/requests for changes directly with ReviewNB app but I'm not sure how to transfer to the formal review, so I'm going to copy them below. Let me know if anything is unclear!
Goals:
- NSF's NOIRLab --> NSF NOIRLab
- After the list of use-case notebooks, we could add a small section with How-to notebooks to include the ones that Ben wrote to display spectra retrieved with SPARCL using JDAViz and using Prospect.
Auth for DL:
- You may want to either comment out the "ac.whoAmI()" command in this cell or run as "demo" so it shows a demo username.
Auth for SPARCL:
- "SSO user name" needs more information to specify that this is the NOIRLab CSDC SSO and with a link to where to create one if necessary: https://sso.csdc.noirlab.edu/
- I think it's a great idea to mention to contact us here if there are problems!
View which data sets you have access to:
- maybe a parenthesis after to explain: "View which data sets you can access (Note: if you are not logged in or if your SSO email is not in the authorized list of SPARCL users, you will only see public datasets):"
@stephjuneau added your edits! Can you review again please? |
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.
Looks great! Two last minor comments....
@@ -7785,7 +7778,7 @@ <h4 id="We-now-create-a-SPARCL-client-instance:">We now create a SPARCL client i | |||
<div class="jp-InputArea jp-Cell-inputArea"><div class="jp-InputPrompt jp-InputArea-prompt"> | |||
</div><div class="jp-RenderedHTMLCommon jp-RenderedMarkdown jp-MarkdownOutput" data-mime-type="text/markdown"> | |||
<p><a class="anchor" id="auth_sparcl"></a></p> | |||
<h1 id="Authentication-for-SPARCL">Authentication for SPARCL<a class="anchor-link" href="#Authentication-for-SPARCL">¶</a></h1><p>All public SPARCL data sets can be accessed without explicitly logging in. However, some data sets are private and can only be accessed by authorized users. If you are an authorized user and wish to login to SPARCL, un-comment the cell below and enter your SSO user name and password.</p> | |||
<h1 id="Authentication-for-SPARCL">Authentication for SPARCL<a class="anchor-link" href="#Authentication-for-SPARCL">¶</a></h1><p>All public SPARCL data sets can be accessed without explicitly logging in. However, some data sets are private and can only be accessed by authorized users. If you are an authorized user and wish to login to SPARCL, un-comment the cell below and enter your NOIRLab CSDC SSO user name and password. If you need to create an account, sign-up at <a href="https://sso.csdc.noirlab.edu/">https://sso.csdc.noirlab.edu/</a> then email datalab-spectro@noirlab.edu with your First Name, Last Name, and Email Address (the same one you used to create your NOIRLab CSDC SSO account).</p> |
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.
"If you need to create an account, sign-up at https://sso.csdc.noirlab.edu." is great but the second part could be misleading because we will already populate the list of authorized SPARCL users (unless there is an issue and a user registers for SSO with different credentials). So let's revise the rest to something like: "If you encounter an issue, email datalab-spectro@noirlab.edu with your First Name, Last Name, and Email Address (the same one you used to create your NOIRLab CSDC SSO account)."
<p>And our other How-To notebooks that use SPARCL:</p> | ||
<ul> | ||
<li><a href="https://github.com/astro-datalab/notebooks-latest/blob/master/04_HowTos/SPARCL/Plot_SPARCL_Spectra_with_Jdaviz.ipynb">SPARCL + Jdaviz</a></li> | ||
<li><a href="https://github.com/astro-datalab/notebooks-latest/blob/master/04_HowTos/SPARCL/Plot_SPARCL_Spectra_with_Prospect.ipynb">Obtain and plot spectra data using SPARCL, prospect, and specutils</a></li> |
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.
I suggest making the notebook short descriptions more homogenous, such as: "Obtain spectra with SPARCL and plot them with Jdaviz" and "Obtain spectra with SPARCL and plot them with prospect" (I think specutils is an intermediary step for converting the data format).
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.
It looks good to go. Thank you!
See DLS-526
@stephjuneau can you please review this NB? You can use the test username and password (I'll DM the credentials to you again if you need)