Skip to content

Exoplanet phase curve#49

Open
arjunsavel wants to merge 6 commits into
spacetelescope:mainfrom
arjunsavel:exoplanet_phase_curve
Open

Exoplanet phase curve#49
arjunsavel wants to merge 6 commits into
spacetelescope:mainfrom
arjunsavel:exoplanet_phase_curve

Conversation

@arjunsavel

Copy link
Copy Markdown
Contributor

No description provided.

@arjunsavel

Copy link
Copy Markdown
Contributor Author

@ttdu @snbianco ready for review!

@snbianco snbianco 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.

This is a great notebook with some really good science! I think a lot of people will find it helpful. Here are some notes I had exclusively on the code cells:

  • There was recently another release of Astroquery (0.4.9) that should have all of the relevant functionality. @ttdu Can we update the version on TIKE's environment?
  • The standard order for Python imports is standard library imports, third party imports, and then local imports. I think the only standard library imports are os, multiprocessing, and concurrent.futures, so I would move those to the top of the cell and then make the spacing a bit more even.
  • There are a few places where you are listing parameters and leave the comma at the start of the newline. Better style is to leave it at the end of the previous line.
  • Instead of filtering the products to only include obs_ids that contain the TIC ID, you could select for the TIC ID in the query using targetname=<TIC>
  • When acquiring lightcurves, I think it would be good to display the results at intermediate steps, i.e., the observations table, the product table, the filtered products, etc.
  • Remember to be consistent with whitespace. It's tricky since it has no effect on the code output, but having clean code style will make for a better reader experience. One line of whitespace is usually enough to separate code blocks.
  • I  like to use plt.show() at the end of blocks that output plots to avoid outputting the matplotlib object.
  • Can you add some comments about the variables you define at the beginning of the sinusoid section? Just a quick note on what they represent.
  • Remember to remove any code that's commented out.
  • The first cell in the MCMC section doesn't have anything to execute. I would just move that comment into the following cell.
  • Some lines have semicolons at the end that I think are unneeded.

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