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

DM-21166: Create DiaCalculation plugins that replicate current ap_association behavior #56

Merged
merged 11 commits into from Oct 15, 2019

Conversation

morriscb
Copy link
Contributor

@morriscb morriscb commented Oct 7, 2019

No description provided.

DataFrame representing diaSources associated with this
diaObject that are observed in the band pass ``filterName``.
filterName : `str`
Simple, string name of the filter for the flux being calculated.
Copy link

Choose a reason for hiding this comment

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

What does simple mean here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Simple as in the string name like "g" or "r". Usually they are single characters but I don't think they have to be.

filterName : `str`
Simple name of the filter for the flux being calculated.
error : `BaseException`
Error to pass.
Copy link

Choose a reason for hiding this comment

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

Probably helpful to mention the default value here.

Copy link

Choose a reason for hiding this comment

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

(And for all the other instances of this)

Debug unittests.

Debug sigma plugin.

Fix up docs
Debug skew.
Debug linear fit.

Change plugin order.

Fix linting
Debug stetsonJ plugin.
Debug tot flux measurements.

Add None to docs.

Fix name of input column for Chi2.

@register("ap_percentileFlux")
class PercentileDiaPsFlux(DiaObjectCalculationPlugin):
"""
Copy link

Choose a reason for hiding this comment

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

Is there a doc string missing here?

DataFrame representing diaSources associated with this
diaObject that are observed in the band pass ``filterName``.
filterName : `str`
Simple, string name of the filter for the fluxb eing calculated.
Copy link

Choose a reason for hiding this comment

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

Typo

Simple, string name of the filter for the flux being calculated.
"""
if len(filterDiaFluxes) > 0:
diaObject["{}PSFluxChi2".format(filterName)] = np.nansum(
Copy link

Choose a reason for hiding this comment

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

This might be easier to read if it is variables in the equation.

if len(filterDiaFluxes) > 0:
pTiles = np.nanpercentile(filterDiaFluxes["psFlux"],
self.config.percentiles)
for pTile, tilePercent in zip(pTiles, self.config.precentiles):
Copy link

Choose a reason for hiding this comment

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

precentile or percentile?

@@ -150,5 +231,6 @@ def fail(self, diaObject, filterName, error=None):
error : `BaseException`
Error to pass.
"""
diaObject["%sPSFluxMean" % filterName] = np.nan
diaObject["%sPSFluxMeanErr" % filterName] = np.nan
for pTile in self.config.precentiles:
Copy link

Choose a reason for hiding this comment

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

precentile

Starting mean value or None.
alpha : `float`
Scalar down-weighting of the fractional difference. lower->more
clipping
Copy link

Choose a reason for hiding this comment

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

It would be helpful to include all the defaults here.

"midPointTai": times})

plug = StetsonJDiaPsFlux(LinearFitDiaPsFluxConfig(),
"ap_LinearFit",
Copy link

Choose a reason for hiding this comment

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

Should this be changed to Stetson things?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, the StetsonJ statistic is the only thing stored.

Debug WeightedMean plugin.

Address last reviewer comments.
@morriscb morriscb merged commit 12a62c0 into master Oct 15, 2019
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