can.io: Add preferred mode for opening files to Reader/Writer classes. by ro-i · Pull Request #1585 · hardbyte/python-can

Conversation

@ro-i

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!

@zariiii9003

Could you explain, which problem you are trying to solve with the PREFERRED_MODE class attribute?

@ro-i

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_MODE

Previously, 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.

@zariiii9003

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"

@ro-i

Thanks for the feedback! I adapted the code (and also considered the *Reader class). Did you have something like that in mind?

zariiii9003



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

zariiii9003

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

2 participants

@ro-i @zariiii9003