Git and CI infrastructure

[kmuehlbauer]

While contributing to and maintaining different open source packages I really got used to tools like pre-commit and lint/style checkers like ruff or black.

Currently there is some lint/style checks within the CI using flake8. This could be used as a blueprint for a dedicated lint/style CI run.

Would this be something which pyfive could also adopt?

[valeriupredoi]

most definitely! I'll add support for pre-commit with Ruff, very very soon, @kmuehlbauer - a very good point indeed! 🍺

[valeriupredoi]

@kmuehlbauer we now have support for pre-commit with Ruff and mypy via #173 - there is still room for improvement by turning off some of the rules we ignore now (they're really not many) but flake8 is now off and we are in 21st century Python standards 😀

[bnlawrence]

@valeriupredoi I'm not enjoying this use of ruff and mypy, they complain a lot about a common pattern, and the only fix is a lot of defensive if statements, which are only the for ruff/mypy, not needed for the code. Even AI writes this common pattern and agree that the semantics it introduces are the correct ones.

The pattern is the one where you instantiate a class with a an attribute set to be None, but later on you will replace that with a list or dictionary in a "first call" method ... other methods can't be called before the first call method, but then want to tests on empty dictionary or lists ... but do not want to do a test on None ... An easy example is a file buffer, where the first thing that happens is a seek, at which point we fill a buffer, and all other methods treat that buffer as an object with length. No other methods can be called before that first seek, and if they were, it would be proper bug higher up in the stack. I don't want to have to do, for everything a check to see that a thing is not None before I do a test on it's length.

[bnlawrence]

Here's a concrete example. In this case self.buffer is set to None in the init method.

 def read(self, size: int = -1) -> bytes:
        """Read from buffer or fall through to original fh."""
        self._ensure_buffer()

        # Calculate buffer bounds
        buffer_end = self.buffer_start + self.buffer.getbuffer().nbytes

ensure_buffer does what you might expect it to do, it makes sure that the buffer exists ... but the type checking stuff doesn't know that and complains that you can't do a getbuffer on None ...

[valeriupredoi]

@bnlawrence this has a very easy fix: we just add the rule code to the ignores in pyproject.toml - what's the barf code?

[bnlawrence]

As discussed, that's not really the solution, but maybe we need to change this pattern throughout to an alternative pattern based on using a local reference to the object returned by the "ensure" method ... here's an example of how it might work for this particular example:

def _ensure_buffer(self) -> io.Bytes:
   if self.buffer is None:
       #... do stuff ...
    return self.buffer

then

def read((self, size: int = -1) -> bytes:
        """Read from buffer or fall through to original fh."""
        buffer = self._ensure_buffer()
        # use buffer in this method instead of self.buffer

At some future time I think we should try and do this throughout the offending code, in particular the code using the chunk index (which I think is all code I wrote, so I think I probably own the problem).