Skip to content

tickets/SP-2913: new tutorial on SSObject image stamps#160

Merged
sgreenstreet merged 4 commits into
mainfrom
tickets/SP-2913
Jun 18, 2026
Merged

tickets/SP-2913: new tutorial on SSObject image stamps#160
sgreenstreet merged 4 commits into
mainfrom
tickets/SP-2913

Conversation

@sgreenstreet

Copy link
Copy Markdown
Contributor

No description provided.

@review-notebook-app

Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@@ -0,0 +1,1477 @@
{

@christinawilliams christinawilliams Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should the image types be in quotes?


Reply via ReviewNB

@@ -0,0 +1,1477 @@
{

@christinawilliams christinawilliams Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some of the subsections, the title runs off the table of contents view because they are long. Suggest perhaps removing "of interest" part here (and for 2.1 remove "for a solar system object..." and 3.1 and 4.1 change to "query the obscore table for first visit / all visits" or something like this


Reply via ReviewNB

@@ -0,0 +1,1477 @@
{

@christinawilliams christinawilliams Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ID doens't match the one above or in the query.


Reply via ReviewNB

@@ -0,0 +1,1477 @@
{

@christinawilliams christinawilliams Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be "store the mpcDesignation" instead of ssObjectId? These IDs are different right? Since you refer tot he mpc ID later, perhaps introduce here what the name is


Reply via ReviewNB

@@ -0,0 +1,1477 @@
{

@christinawilliams christinawilliams Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to orient people by noting that this is done using the visit and detector IDs for the visit that was identified in the last query?

Also suggest defining IVOA upon first use (or in intro)


Reply via ReviewNB

@@ -0,0 +1,1477 @@
{

@christinawilliams christinawilliams Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest making the plots a little larger so that the coordinates don't overlap and are legible


Reply via ReviewNB

@@ -0,0 +1,1477 @@
{

@christinawilliams christinawilliams Jun 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should SS here be lower case (above it is lower case)? Suggest making all references consistent with the official one. Also looks like there's a typo ( iamge -> image)


Reply via ReviewNB

@@ -0,0 +1,1477 @@
{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And also delete the files inside?


Reply via ReviewNB

@@ -0,0 +1,1477 @@
{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add commented text to the markdown


Reply via ReviewNB

@@ -0,0 +1,1477 @@
{

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest delete this last empty cell


Reply via ReviewNB

@christinawilliams christinawilliams left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good! Nice job!

@sgreenstreet sgreenstreet merged commit e34d5c8 into main Jun 18, 2026
2 checks passed
@sgreenstreet sgreenstreet deleted the tickets/SP-2913 branch June 18, 2026 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants