can.io: Add preferred mode for opening files to Reader/Writer classes. by ro-i · Pull Request #1585 · hardbyte/python-can
Conversation
Hi :)
In order to be able to enhance the Logger facility with custom classes, we need a way to handle the preferred file opening modes of those classes. I enhanced the current log writer and reader classes so that each class can specify an own preferred file opening mode which is respected by Logger and LogReader when handling gzip-compressed files.
Additionally, I fixed a small typo leading to several test failures.
Please review and thank you very much for your work!
Sure! I wanted to create own subclasses of MessageWriter/MessageReader. They need to open the underlying files in binary mode. However, the Logger and LogReader classes had hardcoded the file modes to open gzipped files. For example in Logger.compress:
if kwargs.get("append", False):
- mode = "ab" if real_suffix == ".blf" else "at"
+ mode = LoggerType.PREFERRED_MODE_APPEND
else:
- mode = "wb" if real_suffix == ".blf" else "wt"
+ mode = LoggerType.PREFERRED_MODEPreviously, it was hardcoded that only files for the .blf suffix would be opened in binary mode. Now, every MessageReader/MessageWriter can specify its own preferred file opening mode.
I understand now. But we just need to know, whether the log format is binary or not, right?
What do you think about adding these new types to generic.py:
class TextIOMessageWriter(FileIOMessageWriter, metaclass=ABCMeta): file: typing.TextIO class BinaryIOMessageWriter(FileIOMessageWriter, metaclass=ABCMeta): file: typing.BinaryIO | gzip.GzipFile
Then the gzip-mode could just be dependent on a subclass check:
if issubclass(logger_type, BinaryIOMessageWriter): mode = "ab" if append else "wb" else: mode = "at" if append else "wt"
Thanks for the feedback! I adapted the code (and also considered the *Reader class). Did you have something like that in mind?
|
|
||
|
|
||
| class TextIOMessageWriter(FileIOMessageWriter, metaclass=ABCMeta): | ||
| file: TextIO | gzip.GzipFile |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GzipFile returns a TextIOWrapper when opened in text mode, so this should be TextIO only
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀 Thank you for your contribution
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters