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

Add printout of data points; improve titles and labels #211

Merged
merged 4 commits into from
Dec 7, 2020

Conversation

evevkovacs
Copy link
Contributor

If you are submitting a PR for a validation test (either adding a new one or updating an existing one), please post a link to the DESCQA web interface for the test you are submitting.
I added a printout of the data points shown in the plots at the request of the WL group who are cross checking with their results and changed the cosmetics to improve the presentation. See https://portal.nersc.gov/cfs/lsst/descqa/v2/?run=2020-12-04_23&test=shear for an example

@evevkovacs evevkovacs requested a review from yymao December 7, 2020 15:24
Copy link
Member

@yymao yymao left a comment

Choose a reason for hiding this comment

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

Thanks @evevkovacs --- I have some change requests that I commented inline.

@@ -206,26 +211,26 @@ def get_catalog_data(gc, quantities, filters=None):

# define theory from within this class

def post_process_plot(self, ax):
def post_process_plot(self, ax, fig):
Copy link
Member

Choose a reason for hiding this comment

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

To maintain compatibility with other calls of this function..

Suggested change
def post_process_plot(self, ax, fig):
def post_process_plot(self, ax, fig=None):

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I already caught this one myself and fixed it.


fig.subplots_adjust(hspace=0)
Copy link
Member

Choose a reason for hiding this comment

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

To maintain compatibility with other calls of this function..

Suggested change
fig.subplots_adjust(hspace=0)
if fig is not None:
fig.subplots_adjust(hspace=0)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fig should have been in the call to the function. I just forgot it.

'''
Post-processing routines on plot
'''
zmeans = np.linspace(self.zlo, self.zhi, self.ntomo+2)[1:-1]
#zmeans = np.linspace(self.zlo, self.zhi, self.ntomo+2)[1:-1]
Copy link
Member

Choose a reason for hiding this comment

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

Remove unused code instead of just commenting it out. If it must be kept, provide a reason inline.

Suggested change
#zmeans = np.linspace(self.zlo, self.zhi, self.ntomo+2)[1:-1]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

@@ -50,8 +50,11 @@ def __init__(self,
**kwargs):
#pylint: disable=W0231

plt.rcParams['font.size'] = 9

#plt.rcParams['font.size'] = 9
Copy link
Member

Choose a reason for hiding this comment

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

Remove unused code instead of just commenting it out. If it must be kept, provide a reason inline.

Suggested change
#plt.rcParams['font.size'] = 9

@evevkovacs
Copy link
Contributor Author

@yymao The changes are pushed. Sorry I should have checked and cleaned up the code before requesting your review.

Copy link
Member

@yymao yymao left a comment

Choose a reason for hiding this comment

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

Thanks for the update.

@evevkovacs evevkovacs merged commit 122ade4 into master Dec 7, 2020
@evevkovacs evevkovacs deleted the shear_update branch December 7, 2020 16:29
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.

None yet

2 participants