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.

Sign up for GitHub

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

Jump to bottom

DM-15514: Write insertFakeObjectsTask to add fake sources for QA purposes #140

Merged
merged 1 commit into from
Jun 6, 2019
Merged

DM-15514: Write insertFakeObjectsTask to add fake sources for QA purposes #140

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 12 additions & 1 deletion python/lsst/meas/base/forcedPhotCoadd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ class ForcedPhotCoaddConfig(ForcedPhotImageConfig):
optional=True
)

hasFakes = lsst.pex.config.Field(
dtype=bool,
default=False,
doc="Should be set to True if fake sources have been inserted into the input data."
)

def setDefaults(self):
ForcedPhotImageTask.ConfigClass.setDefaults(self)
# Copy 'id' and 'parent' columns without renaming them; since these are
Expand Down Expand Up @@ -88,7 +94,12 @@ class ForcedPhotCoaddTask(ForcedPhotImageTask):
dataPrefix = "deepCoadd_"

def getExposure(self, dataRef):
name = self.config.coaddName + "Coadd_calexp"

if self.config.hasFakes:
name = "fakes_" + self.config.coaddName + "Coadd_calexp"
Copy link
Contributor

Choose a reason for hiding this comment

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

In general the python community seems to be going toward using the format method or f strings instead of the addition operator, so it might be useful to get in the habit. This code would then be

name = f"fakes_{self.config.coaddName}Coadd_calex"

What you have will run fine, just using the opportunity to maybe pass along some tips. There is a runtime performance you gain by using f strings or format vs concatenation, but in this case the micro optimization does not really matter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it preferable to use this way or to have a consistent formatting throughout the file?

Copy link
Contributor

Choose a reason for hiding this comment

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

https://developer.lsst.io/coding/intro.html?highlight=consistency#deviating-from-the-dm-style-guides (second bullet):

Coding consistency is very important but sometimes the style guide doesn’t apply either due to lack of a definitive rule or the circumstances of the specific code segment logically dictate otherwise. When in doubt, use your best judgment or ask the lead developer.

Here are two plausible reasons to break a particular rule:

  • When applying the rule would make the code less readable, even for someone who is used to reading code that follows the rules.
  • To be consistent with surrounding code that also breaks it (maybe for historic reasons)—although this is also an opportunity to clean up someone else’s mess.

else:
name = self.config.coaddName + "Coadd_calexp"

return dataRef.get(name) if dataRef.datasetExists(name) else None

def makeIdFactory(self, dataRef):
Expand Down