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-6999: Logging framework migration #47

Merged
merged 3 commits into from Sep 2, 2016
Merged

DM-6999: Logging framework migration #47

merged 3 commits into from Sep 2, 2016

Conversation

hsinfang
Copy link

@hsinfang hsinfang commented Aug 5, 2016

No description provided.

@isullivan
Copy link
Contributor

It's slightly outside the scope of this pull request, but there are a few lines in these files that could really use to be cleaned up:
in testFlagHandler.py:

  • There are a lot of imported packages that are not used
  • line 130 there is a "if not X is None:" statement that should be "if X is not None"
  • line 131 math is used but not imported

in noiseReplacer.py:

  • line 259 there is another "if not X is None:" statement that should be "if X is not None"
  • lines 191 and 216 both lines have a "not X in Y" statement that should be "X not in Y"

@isullivan
Copy link
Contributor

The code changes all look fine. I don't have the new lsst.log so the changed tests all fail for me, but assuming that they do all pass with the new framework this branch can be merged.

@@ -34,6 +34,7 @@
import numpy as np

import lsst.pex.logging as pexLogging
from lsst.log import Log
import lsst.pex.exceptions
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you forget to remove this, and the code below that uses it? (I didn't see any other instances, but suggest a global search to be sure).

Copy link
Author

Choose a reason for hiding this comment

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

Log is used in line 53 below, also added by this commit.

However, I actually cannot find any logs in afw that logs to afwDetection.Measure. It's likely for some old codes that have been replaced? I will delete these debugging placeholders.

@hsinfang
Copy link
Author

Thanks for the pytest work, testFlagHandler.py and noiseReplacer.py have been cleaned up! :-)

@hsinfang hsinfang force-pushed the tickets/DM-6999 branch 3 times, most recently from a11d8f9 to 886976b Compare August 31, 2016 22:01
Hsin-Fang Chiang added 3 commits August 31, 2016 17:02
- Replace pex.logging logdebug using lsst.log.Log DEBUG level
- Remove some unused pex.logging dependency
Currently there is not a logger named "afwDetection.Measure" in afw.
@hsinfang hsinfang merged commit 1f1f621 into master Sep 2, 2016
@ktlim ktlim deleted the tickets/DM-6999 branch August 25, 2018 06:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants