Enhance `can.logger` to consider the `append` option #1327

j-c-cook · 2022-06-06T15:53:17Z

The goal here appears to be to give access to the boolean append option of Printer.

Lines 16 to 40 in 0d7f65f

    
           class Printer(MessageWriter): 
        
               """ 
        
               The Printer class is a subclass of :class:`~can.Listener` which simply prints 
        
               any messages it receives to the terminal (stdout). A message is turned into a 
        
               string using :meth:`~can.Message.__str__`. 
        
               :attr write_to_file: `True` if this instance prints to a file instead of 
        
                                    standard out 
        
               """ 
        
               file: Optional[TextIO] 
        
               def __init__( 
        
                   self, file: Optional[Union[StringPathLike, TextIO]] = None, append: bool = False 
        
               ) -> None: 
        
                   """ 
        
                   :param file: An optional path-like object or a file-like object to "print" 
        
                                to instead of writing to standard out (stdout). 
        
                                If this is a file-like object, is has to be opened in text 
        
                                write mode, not binary write mode. 
        
                   :param append: If set to `True` messages, are appended to the file, 
        
                                  else the file is truncated 
        
                   """ 
        
                   mode = "a" if append else "w" 
        
                   super().__init__(file, mode=mode)

closes #1326

codecov · 2022-06-06T15:55:10Z

Codecov Report

Merging #1327 (e17a9f8) into develop (0c4dee0) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #1327   +/-   ##
========================================
  Coverage    65.96%   65.97%           
========================================
  Files           86       86           
  Lines         8953     8955    +2     
========================================
+ Hits          5906     5908    +2     
  Misses        3047     3047

j-c-cook · 2022-06-06T16:41:05Z

Interestingly it looks like Python's argparser.py isn't well suited for a bool option in Python==3.7.13.

The following line is where the issue occurs:

 # convert the value to the appropriate type
        try:
            result = type_func(arg_string)

The reason for the issue can be explained below:

>>> print(bool(0))
False
>>> print(bool(1))
True
>>> print(bool('0'))
True
>>> print(bool('1'))
True

The command line arguments are passed in as strings, and when evaluating bool(<str>) the result is always True.

This solution is only required until Python>=3.9.

j-c-cook · 2022-06-06T17:15:11Z

can/logger.py

+    # argparser.py in Python 3.7.11 does not properly consider boolean arguments passed
+    # in from the command line. If a boolean is passed in, the variable must be converted
+    # to an integer 0 (False) or 1 (True). Once the project goes to a minimum of
+    # Python 3.9, the BooleanOptionalAction, this will no longer be required (Please do
+    # not use this as a reason to advocate a minimum Python version of 3.9).
+    boolean_args = ['-a', '--append']
+    args = sys.argv[1:]
+    for i, bool_arg in enumerate(boolean_args):
+        for j, arg in enumerate(args):
+            if bool_arg == arg:
+                # Make sure the length of args is long enough to check the action
+                if len(args) >= j+1:
+                    action = args[j+1]
+                else:
+                    break
+                # Consider the action, and update the results to accurately embody request
+                if action == '0' or action == 'False':
+                    results.append_mode = False
+                elif action == '1' or action == 'True':
+                    results.append_mode = True
+                else:
+                    pass


I added a manual argparse to search for boolean_args and correct the value of results.append_mode.

NOTE: Only required until Python>=3.9
NOTE: Please do not use this as a reason to advocate Python>=3.9.

j-c-cook · 2022-06-06T17:30:09Z

@hardbyte @zariiii9003 This accomplishes what was described in #1326 and is ready for review.

zariiii9003 · 2022-06-07T08:11:28Z

Could you fix the linters?

j-c-cook · 2022-06-07T14:14:42Z

@zariiii9003 The *lack format is failing here (https://github.com/hardbyte/python-can/runs/6761006357?check_suite_focus=true), but when I run it locally the checks pass.

$ black --check --verbose logger.py
Identified `/home/user/Documents/python-can` as project root containing a .git directory.
Sources to be formatted: "can/logger.py"
logger.py wasn't modified on disk since last run.

All done! ✨ 🍰 ✨
1 file would be left unchanged.

zariiii9003 · 2022-06-07T14:17:25Z

I guess you are using a different black version. Run pip install -r requirements-lint.txt

j-c-cook · 2022-06-07T14:17:51Z

Could you fix the linters?

@zariiii9003 Done. I'm passing 24/25 checks.

j-c-cook · 2022-06-07T14:52:38Z

@zariiii9003 Do you want for me to add in tests to try to resolve the https://github.com/hardbyte/python-can/pull/1327/checks?check_run_id=6775694757 failure?

zariiii9003 · 2022-06-08T23:24:02Z

can/logger.py

@@ -201,12 +219,42 @@ def main() -> None:
    print(f"Connected to {bus.__class__.__name__}: {bus.channel_info}")
    print(f"Can Logger (Started on {datetime.now()})")

+    # argparser.py in Python 3.7.11 does not properly consider boolean


You don't need any of this. Look at how the --fd argument is handled. It is also a boolean.

I agree. I have modified my changes. This simplifies things. Thank you.

can/logger.py

j-c-cook · 2022-06-09T15:45:20Z

@zariiii9003 I committed your recommendations. Next time you do that I'll make sure to accept them from this UI rather than manually change them.

Add passage of **options to the logger initialization

7866362

Add -a boolean option to the parser

7312246

Add manual arg parse for boolean variables

b2c3ee4

This solution is only required until Python>=3.9.

j-c-cook commented Jun 6, 2022

View reviewed changes

Fix the index-error check

24aae13

j-c-cook marked this pull request as ready for review June 6, 2022 17:29

j-c-cook added 2 commits June 6, 2022 12:33

Bolster help documentation for new -a option

7a10157

Clean up variable names, notes, etc.

bad8a87

j-c-cook added 3 commits June 7, 2022 08:30

Whitespace formatting (PEP-8 style)

3446ee4

Make formatting recomendations based on pylint logger.py

7990ff5

Format code with black logger.py to stop failing the black check

1cf0e73

Merge branch 'develop' into issue1326_LoggerWriteMode

ac8dc4e

zariiii9003 reviewed Jun 8, 2022

View reviewed changes

j-c-cook added 2 commits June 8, 2022 20:37

Reduce complexity of append arg parse incorporation

78a305d

Fix format with black can/logger.py

318d7b4

zariiii9003 requested changes Jun 9, 2022

View reviewed changes

can/logger.py Outdated Show resolved Hide resolved

can/logger.py Show resolved Hide resolved

can/logger.py Outdated Show resolved Hide resolved

j-c-cook added 2 commits June 9, 2022 10:36

Change append argparse access from results.a to results.append

2c75189

Add dest option to append argument

e17a9f8

j-c-cook requested a review from zariiii9003 June 9, 2022 15:46

zariiii9003 approved these changes Jun 10, 2022

View reviewed changes

zariiii9003 merged commit 1430154 into hardbyte:develop Jun 10, 2022

j-c-cook mentioned this pull request Jun 10, 2022

Enhance the can.logger to check to see if the log file exists and append if there rather than overwrite #1326

Closed

j-c-cook deleted the issue1326_LoggerWriteMode branch August 11, 2022 18:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhance `can.logger` to consider the `append` option #1327

Enhance `can.logger` to consider the `append` option #1327

j-c-cook commented Jun 6, 2022 •

edited

Loading

codecov bot commented Jun 6, 2022 •

edited

Loading

j-c-cook commented Jun 6, 2022

j-c-cook Jun 6, 2022 •

edited

Loading

j-c-cook commented Jun 6, 2022 •

edited

Loading

zariiii9003 commented Jun 7, 2022

j-c-cook commented Jun 7, 2022

zariiii9003 commented Jun 7, 2022

j-c-cook commented Jun 7, 2022 •

edited

Loading

j-c-cook commented Jun 7, 2022

zariiii9003 Jun 8, 2022

j-c-cook Jun 9, 2022

j-c-cook commented Jun 9, 2022

	class Printer(MessageWriter):
	"""
	The Printer class is a subclass of :class:`~can.Listener` which simply prints
	any messages it receives to the terminal (stdout). A message is turned into a
	string using :meth:`~can.Message.__str__`.

	:attr write_to_file: `True` if this instance prints to a file instead of
	standard out
	"""

	file: Optional[TextIO]

	def __init__(
	self, file: Optional[Union[StringPathLike, TextIO]] = None, append: bool = False
	) -> None:
	"""
	:param file: An optional path-like object or a file-like object to "print"
	to instead of writing to standard out (stdout).
	If this is a file-like object, is has to be opened in text
	write mode, not binary write mode.
	:param append: If set to `True` messages, are appended to the file,
	else the file is truncated
	"""
	mode = "a" if append else "w"
	super().__init__(file, mode=mode)

Enhance can.logger to consider the append option #1327

Enhance can.logger to consider the append option #1327

Conversation

j-c-cook commented Jun 6, 2022 • edited Loading

codecov bot commented Jun 6, 2022 • edited Loading

Codecov Report

j-c-cook commented Jun 6, 2022

j-c-cook Jun 6, 2022 • edited Loading

Choose a reason for hiding this comment

j-c-cook commented Jun 6, 2022 • edited Loading

zariiii9003 commented Jun 7, 2022

j-c-cook commented Jun 7, 2022

zariiii9003 commented Jun 7, 2022

j-c-cook commented Jun 7, 2022 • edited Loading

j-c-cook commented Jun 7, 2022

zariiii9003 Jun 8, 2022

Choose a reason for hiding this comment

j-c-cook Jun 9, 2022

Choose a reason for hiding this comment

j-c-cook commented Jun 9, 2022

Enhance `can.logger` to consider the `append` option #1327

Enhance `can.logger` to consider the `append` option #1327

j-c-cook commented Jun 6, 2022 •

edited

Loading

codecov bot commented Jun 6, 2022 •

edited

Loading

j-c-cook Jun 6, 2022 •

edited

Loading

j-c-cook commented Jun 6, 2022 •

edited

Loading

j-c-cook commented Jun 7, 2022 •

edited

Loading