-
Notifications
You must be signed in to change notification settings - Fork 51
New galaxy clusters modeling notebook (based on SMACS J0723.3-7327, Mahler et al. 2023) #41
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
base: main
Are you sure you want to change the base?
Conversation
…327 Mahler et al. 2023 data) and the data files needed to run the notebook.
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| @@ -0,0 +1,1965 @@ | |||
| { | |||
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.
might need more top-level information. Indicate what products are being used for this notebook, perhaps even in what format
Reply via ReviewNB
| @@ -0,0 +1,1965 @@ | |||
| { | |||
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.
do we still need these descriptions? Try not to duplicate too many feature of another notebook but instead link to that other notebook (easier to maintain fewer information per notebook)
Reply via ReviewNB
| @@ -0,0 +1,1965 @@ | |||
| { | |||
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 I would provide more text, the file name and structure, perhaps even the link to the paper
Reply via ReviewNB
| @@ -0,0 +1,1965 @@ | |||
| { | |||
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.
Line #33. for i in range(len(gal_data_deg)):
you have many different for-loops here operating on the same part (coordinates). Might be better to do only one for-loop and do all the things at once, perhaps having a definition that does it
Reply via ReviewNB
| @@ -0,0 +1,1965 @@ | |||
| { | |||
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.
Line #107. # :param center_y: center of halo (in angular units)
do we need all the prints here? It's hard to read it anyways
Reply via ReviewNB
Upload of new galaxy clusters modeling notebook based on the work of Mahler et al. 2023 on the SMACS J0723.3-7237. Also added the relevant data files (arcs.dat, galcat.cat, input.par) needed to run the notebook.