Skip to content

Conversation

@multiplor
Copy link

Change-Id: I61a04c53276ea5b74e81d77267ef5e063a211d8a

Change-Id: I61a04c53276ea5b74e81d77267ef5e063a211d8a
Copy link
Collaborator

@felixdivo felixdivo left a comment

Choose a reason for hiding this comment

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

Thank you @multiplor for you contribution!

This generally looks good, although I do not unterstand the benefit of a specific parser exception against for example a simple ValueError. What was the rationale?

@felixdivo felixdivo added the file-io about reading & writing to files label Aug 15, 2019
@felixdivo
Copy link
Collaborator

And both linter and formatter complain, you can run them locally with pylint --rcfile=.pylintrc-wip can/ and black --verbose . after installing the dependencies with pip3 install -r requirements-lint.txt -U.

@felixdivo felixdivo added this to the 4.0 Release milestone Aug 16, 2019
can/io/asc.py Outdated
self.base = BASES[base]
self.absolute_timestamps = m.group('timestamps') == 'absolute'
break
self.file.seek(0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it really necessary to seek? Couldn’t we just parse the header until we hit Begin Triggerblock and then continue with the actual log entries? The reason I’m asking is because there might be cases where one would like to stream the log over e.g. HTTP or something and then it would not be possible to seek.

@codecov
Copy link

codecov bot commented Aug 21, 2019

Codecov Report

Merging #685 into develop will increase coverage by 0.07%.
The diff coverage is 95.65%.

@@             Coverage Diff             @@
##           develop     #685      +/-   ##
===========================================
+ Coverage    68.74%   68.81%   +0.07%     
===========================================
  Files           69       69              
  Lines         6245     6253       +8     
===========================================
+ Hits          4293     4303      +10     
+ Misses        1952     1950       -2

@codecov
Copy link

codecov bot commented Aug 21, 2019

Codecov Report

Merging #685 into develop will decrease coverage by 4.05%.
The diff coverage is 95.65%.

@@             Coverage Diff             @@
##           develop     #685      +/-   ##
===========================================
- Coverage    68.74%   64.68%   -4.06%     
===========================================
  Files           69       69              
  Lines         6245     6153      -92     
===========================================
- Hits          4293     3980     -313     
- Misses        1952     2173     +221

Change-Id: I31169574892448f5df52da21fe563272244d8634
@felixdivo
Copy link
Collaborator

I think this is superseded by #820.

@felixdivo felixdivo closed this Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

file-io about reading & writing to files work-in-progress

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants