[WIP] refactor dict usage (#321) #338
Conversation
Codecov Report
@@ Coverage Diff @@
## development #338 +/- ##
============================================
Coverage 40.31% 40.31%
============================================
Files 37 37
Lines 7023 7023
Branches 1654 1653 -1
============================================
Hits 2831 2831
Misses 3989 3989
Partials 203 203
Continue to review full report at Codecov.
|
src/canmatrix/cli/compare.py
Outdated
| @@ -242,8 +242,7 @@ def compare_attributes(ele1, ele2, ignore=None): | |||
| if ignore is None: | |||
| ignore = dict() | |||
| result = CompareResult("equal", "ATTRIBUTES", ele1) | |||
| if "ATTRIBUTE" in ignore and ( | |||
| ignore["ATTRIBUTE"] == "*" or ignore["ATTRIBUTE"] == ele1): | |||
| if ignore.get("ATTRIBUTE", "") == "*" or ignore.get("ATTRIBUTE","") == ele1: | |||
src/canmatrix/formats/cmjson.py
Outdated
| additionalFrameColums = [] # type: typing.List[str] | ||
| if "additionalFrameAttributes" in options and options["additionalFrameAttributes"]: | ||
| additionalFrameColums = options["additionalFrameAttributes"].split(",") | ||
| if options.get("additionalFrameAttributes",False): |
There was a problem hiding this comment.
please add space after comma (PEP8), or even simplify to single line
additionalFrameColums = options.get("additionalFrameAttributes", "").split(",")
It should be enough, because "".split(",") is exactly []
There was a problem hiding this comment.
eeeeeee sorry, the "".split(",") is NOT [] but [""] so most of my comments are WRONG!
There was a problem hiding this comment.
I was thinking about how to write it on single line, so that the code is more condensated. Maybe this:
additionalFrameColums = [x for x in options.get("additionalFrameAttributes", "").split(",") if x]
src/canmatrix/formats/cmjson.py
Outdated
| @@ -192,10 +193,9 @@ def load(f, **options): | |||
| newframe = canmatrix.Frame(frame["name"], | |||
| arbitration_id=frame["id"], | |||
| size=8) | |||
| if "length" in frame: | |||
| newframe.size = frame["length"] | |||
| newframe.size = frame.get("length",0) | |||
There was a problem hiding this comment.
wait, really zero? Or rather the size=8 from above? I think this code was OK, no need to change.
src/canmatrix/formats/cmjson.py
Outdated
| @@ -221,7 +221,7 @@ def load(f, **options): | |||
| factor=signal["factor"], | |||
| offset=signal["offset"]) | |||
|
|
|||
| if signal.get("min") is not None: | |||
| if signal.get("min", False) is not None: | |||
There was a problem hiding this comment.
reparied it already... to
if signal.get("min", False):| else: | ||
| additional_signal_colums = []#["attributes['DisplayDecimalPlaces']"] | ||
|
|
||
| if "additionalFrameAttributes" in options and len(options["additionalFrameAttributes"]) > 0: | ||
| additional_frame_colums = options["additionalFrameAttributes"].split(",") | ||
| if len(options.get("additionalFrameAttributes","")) > 0: |
| motorolaBitFormat = options["xlsMotorolaBitFormat"] | ||
| else: | ||
| motorolaBitFormat = "msbreverse" | ||
| motorolaBitFormat = options.get("xlsMotorolaBitFormat","msbreverse") |
There was a problem hiding this comment.
space after comma. Generally I think that it is good idea to keep the code style rules, because the IDE (pycharm) marks various issues and if we eliminate such superfluous warning sources, we can focus/see real problems. Of course the codestyle check can be disabled, but... yes the python freedom has some costs. But of course it is not my codebase, feel free to ignore me.
| @@ -469,7 +466,7 @@ def load(file, **options): | |||
| mux, signalComment = signalComment[4:].split(':', 1) | |||
| multiplex = int(mux.strip()) | |||
|
|
|||
| if "byteorder" in index: | |||
| if index.get("byteorder", False): | |||
There was a problem hiding this comment.
original code was OK I think
| @@ -123,13 +123,13 @@ def dump(db, filename, **options): | |||
| 'Byteorder'] | |||
| head_tail = ['Value', 'Name / Phys. Range', 'Function / Increment Unit'] | |||
|
|
|||
| if "additionalAttributes" in options and len(options["additionalAttributes"]) > 0: | |||
| additional_signal_colums = options["additionalAttributes"].split(",") | |||
| if len(options.get("additionalAttributes", "")) > 0: | |||
There was a problem hiding this comment.
convert to single line
| else: | ||
| additional_signal_colums = []#["attributes['DisplayDecimalPlaces']"] | ||
|
|
||
| if "additionalFrameAttributes" in options and len(options["additionalFrameAttributes"]) > 0: | ||
| additional_frame_colums = options["additionalFrameAttributes"].split(",") | ||
| if len(options.get("additionalFrameAttributes", "")) > 0: |
There was a problem hiding this comment.
convert to single line
Co-Authored-By: ebroecker <eduard.broecker@gmail.com>
|
@Funth0mas |
No description provided.