Log files updates #165

wtgee · 2017-12-03T06:35:03Z

Add docs
Log file per level
Log files name consistency
Clean up a few instances where it just had warn instead of warning

Replaces #163 which replaces #160. The difference with #163 here is just that it has a cleaner history without extraneous commits.

* Add docs * Log file per level * Log files name consistency * Clean up a few instances where it just had `warn` instead of `warning`

codecov · 2017-12-03T06:49:55Z

Codecov Report

Merging #165 into develop will increase coverage by 0.21%.
The diff coverage is 90.47%.

@@             Coverage Diff             @@
##           develop     #165      +/-   ##
===========================================
+ Coverage     80.6%   80.82%   +0.22%     
===========================================
  Files           36       36              
  Lines         2527     2556      +29     
  Branches       319      320       +1     
===========================================
+ Hits          2037     2066      +29     
- Misses         386      387       +1     
+ Partials       104      103       -1

Impacted Files	Coverage Δ
pocs/mount/mount.py	`57.84% <0%> (ø)`	⬆️
pocs/core.py	`79.82% <66.66%> (-0.09%)`	⬇️
pocs/utils/logger.py	`96.36% <97.29%> (+4.36%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b3929e...418cad3. Read the comment docs.

jamessynge · 2017-12-03T17:56:04Z

CONTRIBUTING.md


-Please see the [code of conduct](https://github.com/panoptes/POCS/blob/develop/CODE_OF_CONDUCT.md) for our
+Please see the [code of conduct](https://github.com/panoptes/POCS/blob/develop/CODE_OF_CONDUCT.md) 


This should probably be a link to the primary branch (which is changeable), so please remove "blob/develop/" from the URL.

Unfortunately those kind of links don't work in github. The blob/develop is the only way to link to an active item in to the repo AFAIK.

jamessynge · 2017-12-03T17:57:47Z

CONTRIBUTING.md

@@ -22,22 +26,84 @@ playground rules and follow them during all your contributions.


 # Setting up Local Environment
-  - Follow instructions on the [Coding in PANOPTES](https://github.com/panoptes/POCS/wiki/Coding-in-PANOPTES).
+  - Follow instructions on the 


and in README.md... which probably means we need to spend some time (not immediately) to organize the docs into sensible locations.

jamessynge · 2017-12-03T17:58:39Z

CONTRIBUTING.md

+instead of `My File.py`.
+- Define any project specific terminology or abbreviations you use in the file you use them.
+- Variable/function/class and file names should be meaningful and descriptive.
+- File names should be underscored, not contain spaces ex. my_file.py.


Two duplicate (older) lines.

Wow, I think I have merged this incorrectly every single time. ;)

jamessynge · 2017-12-03T17:59:55Z

CONTRIBUTING.md

+   continue.
+- The logger supports variable information without the use of the `format` method.
+- There is a `say` method that is meant to be used in friendly manner to convey information to a 
+user. This should be used only for personable output and is typically displayed in the "chat box" 


By "personable output", shall I assume that you mean something like "messages intended for the operator"?

Sort of yes. I made a change to description above. The say method is only available to the POCS class (not in PanBase and therefore not in all classes) and should be very friendly output. As in something that you could feasibly send to twitter if you wanted your unit to do such a thing.

The only place so far that there is really operator output was for the drift alignment stuff and that was simply to support Nem the one time he was doing that. During normal operations the say output doesn't include anything technical.

The method is also a place that could easily be internationalized, which I have an Issue for, which would mostly be for fun. But that way someone can make their unit "speak" in whatever language they want.

jamessynge · 2017-12-03T18:01:25Z

CONTRIBUTING.md

+   - ERROR (i.e. `self.logger.error()`) should be used at critical levels when operation cannot 
+   continue.
+- The logger supports variable information without the use of the `format` method.
+- There is a `say` method that is meant to be used in friendly manner to convey information to a 


There is a say method available on sub-classes of PanBase that is meant...

jamessynge · 2017-12-03T18:02:46Z

pocs/core.py

@@ -43,6 +43,11 @@ def __init__(self, state_machine_file='simple_state_table', messaging=False, **k
        PanBase.__init__(self, **kwargs)

        self.logger.info('Initializing PANOPTES unit')


Remove old, non-parameterized message.

jamessynge · 2017-12-03T18:03:40Z

pocs/images.py

@@ -115,7 +115,8 @@ def pointing_error(self):
            namedtuple: Pointing error information
        """
        if self._pointing_error is None:
-            assert self.pointing is not None, self.logger.warn("No WCS, can't get pointing_error")
+            assert self.pointing is not None, self.logger.warning(
+                "No WCS, can't get pointing_error")


jamessynge · 2017-12-03T18:05:43Z

pocs/utils/logger.py


    # Configure the logger
    logging.config.dictConfig(log_config)

    # Get the logger and set as attribute to class
    logger = logging.getLogger(profile)

+    # Don't want log messages from state machine library


I appreciate the comment as I had no idea what this was about. I wonder if this is the kind of thing that could be in conf_files/log.yaml. What do you think?

I don't think we ever want to show these. There is a lot of output that is generated (something like: prepare, before enter, enter, after enter, before exit, exit, etc) and we already have our own logging for entering/exiting each state.

Got it. In that case, please say so succinctly.

Don't want log messages from state machine library, it is very noisy and we have our own logging of state transitions.

jamessynge · 2017-12-03T18:11:44Z

pocs/utils/logger.py

+        super(PanLogger, self).__init__()
+        self.logger = logger
+
+    def _process_str(self, fmt, *args, **kwargs):


I don't understand why this method is necessary. According to the python documentation, the loggers do exactly this formatting.

This supports the actual format style, e.g. {:0.2f}, {:40s}, rather than just supporting {} as placeholders. The loggers support this via the %f syntax, which is simply for backwards-compatibility. This allows us to just get rid of actual format calls in the logger but still keep the same syntax.

I could definitely be wrong about this but the format specifiers didn't seem to work for me otherwise.

OK. Perhaps add a comment explaining that?

Update now that I'm trying to use the logger: it is confusing to have PanLogger use a different formatting convention from python's loggers, even though the API is otherwise identical.

Not sure I follow. You should be able to use either convention.

…e-updates

Log files updates

fc6ec10

* Add docs * Log file per level * Log files name consistency * Clean up a few instances where it just had `warn` instead of `warning`

wtgee requested a review from jamessynge December 3, 2017 06:35

wtgee mentioned this pull request Dec 3, 2017

Log file updates #163

Closed

Add an entry in the changelog

9349c3b

jamessynge requested changes Dec 3, 2017

View reviewed changes

wtgee added 3 commits December 4, 2017 09:10

Formatting and doc changes

f4b3a54

Merge branch 'develop' into log-file-updates

a0aea91

Merge branch 'develop' into log-file-updates

bb7594d

jamessynge previously approved these changes Dec 4, 2017

View reviewed changes

wtgee added 2 commits December 4, 2017 12:27

Adding comments based upon review

7c398be

Merge branch 'log-file-updates' of github.com:wtgee/POCS into log-fil…

418cad3

…e-updates

wtgee dismissed jamessynge’s stale review via 418cad3 December 4, 2017 01:34

wtgee merged commit cfb1515 into panoptes:develop Dec 4, 2017

wtgee deleted the log-file-updates branch December 4, 2017 03:04

wtgee mentioned this pull request Dec 29, 2017

Release 0.6.0 #271

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Log files updates #165

Log files updates #165

wtgee commented Dec 3, 2017

codecov bot commented Dec 3, 2017 •

edited

Loading

jamessynge Dec 3, 2017

wtgee Dec 3, 2017

jamessynge Dec 4, 2017

jamessynge Dec 3, 2017

jamessynge Dec 3, 2017

wtgee Dec 3, 2017

jamessynge Dec 3, 2017

wtgee Dec 3, 2017

jamessynge Dec 3, 2017

jamessynge Dec 3, 2017

jamessynge Dec 3, 2017

jamessynge Dec 3, 2017

wtgee Dec 3, 2017

jamessynge Dec 4, 2017 •

edited

Loading

jamessynge Dec 3, 2017

wtgee Dec 3, 2017

jamessynge Dec 4, 2017

jamessynge Dec 14, 2017

wtgee Dec 14, 2017


		Please see the [code of conduct](https://github.com/panoptes/POCS/blob/develop/CODE_OF_CONDUCT.md) for our
		Please see the [code of conduct](https://github.com/panoptes/POCS/blob/develop/CODE_OF_CONDUCT.md)

		@@ -43,6 +43,11 @@ def __init__(self, state_machine_file='simple_state_table', messaging=False, **k
		PanBase.__init__(self, **kwargs)

		self.logger.info('Initializing PANOPTES unit')

Log files updates #165

Log files updates #165

Conversation

wtgee commented Dec 3, 2017

codecov bot commented Dec 3, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamessynge Dec 4, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 3, 2017 •

edited

Loading

jamessynge Dec 4, 2017 •

edited

Loading