-
Notifications
You must be signed in to change notification settings - Fork 30
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
Color Redshift DESCQA Test #164
Color Redshift DESCQA Test #164
Conversation
@yymao, could you review this PR--it's ready. I don't see an option to request a code review now (I didn't request one when I submitted it initially) |
Thanks, will do (probably not until later this week though). |
No rush. Thanks! |
@yymao, it would be good to get this branch into master. I'm not sure why the it has failed the travis-ci test. |
descqa/ColorRedshiftTest.py
Outdated
|
||
__all__ = ['ColorRedshiftTest'] | ||
class _CatalogDoesNotHaveQuantity(Exception): | ||
"""Raised when the catalog doesn't have a quantity and indicates the |
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.
descqa/ColorRedshiftTest.py:11:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
descqa/ColorRedshiftTest.py
Outdated
__all__ = ['ColorRedshiftTest'] | ||
class _CatalogDoesNotHaveQuantity(Exception): | ||
"""Raised when the catalog doesn't have a quantity and indicates the | ||
test should be skipped""" |
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.
descqa/ColorRedshiftTest.py:13:0: W0311: Bad indentation. Found 8 spaces, expected 4 (bad-indentation)
@dkorytov Please click on the details link above in the "continuous-integration/travis-ci/pr " box to see the issues. I added a comments to my code review to indicate which needed to be fixed. Only the warnings are fatal. The bulk of the comments are stylistic and optional. Please fix the warnings and commit the code. Travis runs automatically. If all the warnings are cleared (W021 etc) it should pass. Let me know if you have any questions. The warnings are easy to fix and should only take a few minutes. If you could get this done before Friday, that would be great. Thanks |
Just seeing this. If there were a default set of plots done for halo centrals (>3e13 or so) that is also a great thing to inspect. |
@erykoff , There is already a config for M_halo>1e13 and red sequence galaxies (not centrals). But the a test config file can set up for that quite simply. Let me add it now :) |
Here's how the test looks like: https://portal.nersc.gov/project/lsst/descqa/v2/?run=2019-07-18&test=color_z_massive_centrals |
descqa/ColorRedshiftTest.py
Outdated
class _CatalogDoesNotHaveQuantity(Exception): | ||
"""Raised when the catalog doesn't have a quantity and indicates the | ||
test should be skipped""" | ||
def __init__(self, quantity_name): |
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.
descqa/ColorRedshiftTest.py:14:0: W0311: Bad indentation. Found 12 spaces, expected 8 (bad-indentation)
descqa/ColorRedshiftTest.py
Outdated
else: | ||
redshift_block_limit = 3 | ||
else: | ||
filters = None |
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.
descqa/ColorRedshiftTest.py:128:0: W0311: Bad indentation. Found 16 spaces, expected 12 (bad-indentation)
This test plots various color-redshfit diagnostics | ||
""" | ||
def __init__(self, **kwargs): | ||
# load test config options |
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.
Add line (after line 21):
# pylint: disable=W0231
This will suppress travis warnings
This is the initial PR for the color-redshift descqa test. There are couple things that are not quite behaving correctly at the moment though.