Skip to content
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

Cosmos multiband notebook #320

Merged
merged 4 commits into from
Jun 29, 2022
Merged

Conversation

mpaillassa
Copy link
Collaborator

Here is the PR related to #316.

I just added some multiband HSC images using the catalog already there. The next iterations may provide some better looking galaxies but it may be better to stay on the first one so that the user gets the same results.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@mpaillassa mpaillassa changed the base branch from main to develop June 16, 2022 15:43
@mpaillassa
Copy link
Collaborator Author

Oh and I just noticed that the plotting function writes "gri images" no matter what bands we select, so we may want to fix this. Maybe in another PR ?

@codecov
Copy link

codecov bot commented Jun 16, 2022

Codecov Report

Merging #320 (096cb0c) into develop (777fe05) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #320   +/-   ##
========================================
  Coverage    88.99%   88.99%           
========================================
  Files           11       11           
  Lines         1445     1445           
========================================
  Hits          1286     1286           
  Misses         159      159           
Flag Coverage Δ
unittests 88.99% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 777fe05...096cb0c. Read the comment docs.

@ismael-mendoza
Copy link
Collaborator

Oh and I just noticed that the plotting function writes "gri images" no matter what bands we select, so we may want to fix this. Maybe in another PR ?

@thuiop does #323 address this ?

@@ -68,9 +68,7 @@
"id": "975ddc32",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I noticed the names of the files are a bit confusing _fits.fits,is there another name we can use to make it clear the information that each file contains?


Reply via ReviewNB

Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe a different suffix we can use for both catalog files? I think we can change all filenames accordingly if there is a better name you can think of

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes I used those names to follow the convention of the original COSMOS dataset, i.e., catalog.fits for the catalog containing information for real galaxies and catalog_fits.fits for the catalog containing the information for parametric galaxies. The thing is that galsim automatically looks for files named with this convention, so I do not think we can set other names unless we change galsim...
When making our extended galsim COSMOS dataset, I also wondered in which catalog I should put the EL-COSMOS HSC multiband magnitudes (both options are possible because we merge both catalogs in the BTK catalog class) and I think the second one makes sense because those magnitudes come from fits, but I may put them in the first catalog (along with the original HST magnitude of galaxies) if we think it makes more sense.

@ismael-mendoza ismael-mendoza merged commit 6a6c973 into develop Jun 29, 2022
@ismael-mendoza ismael-mendoza deleted the cosmos_multiband_notebook branch June 29, 2022 14:23
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