Skip to content

Adding support for gzipped ASC logging file (.asc.gz) #1138

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

Merged
merged 3 commits into from
Nov 25, 2021
Merged

Conversation

pierreluctg
Copy link
Collaborator

@pierreluctg pierreluctg commented Oct 15, 2021

Eventually it would be nice if we could support other "compressor" (zip, bz2, lzma, tar, etc.) automatically by extension detections. I will leave this to future improvements.

@mergify mergify bot requested a review from hardbyte October 15, 2021 14:54
@pierreluctg pierreluctg added enhancement file-io about reading & writing to files labels Oct 15, 2021
@codecov
Copy link

codecov bot commented Oct 15, 2021

Codecov Report

Merging #1138 (a234bc9) into develop (4db969d) will increase coverage by 0.13%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop    #1138      +/-   ##
===========================================
+ Coverage    70.88%   71.01%   +0.13%     
===========================================
  Files           79       79              
  Lines         7768     7790      +22     
===========================================
+ Hits          5506     5532      +26     
+ Misses        2262     2258       -4     

Copy link
Owner

@hardbyte hardbyte left a comment

Choose a reason for hiding this comment

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

Doesn't Python make that easy!

Looks good to me.

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.

An example (in a test case) would be nice, but still a good feature! 👍

@felixdivo
Copy link
Collaborator

Maybe one could also add a very minimalistic test similar to this. Whoops, there is also a copy-paste error in that line, I will fix that.

@pierreluctg
Copy link
Collaborator Author

pierreluctg commented Oct 26, 2021

Maybe one could also add a very minimalistic test similar to this. Whoops, there is also a copy-paste error in that line, I will fix that.

Thank you for the typo fix.

I am working in a full set of unit tests and few other changes since the current code do not really work with x.y file extensions (https://github.com/hardbyte/python-can/blob/develop/can/io/logger.py#L78 and https://github.com/hardbyte/python-can/blob/develop/can/io/player.py#L78 expect a single extension/suffix).

This is why I kept the work-in-progress label.

This PR is not technically broken without the logger and player changes. But I would like to make it more complete.

@felixdivo
Copy link
Collaborator

Sounds great @pierreluctg!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement file-io about reading & writing to files
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants