MartinThoma / flake8-simplify

❄ A flake8 plugin that helps you to simplify code
MIT License
185 stars 19 forks source link

SIM115: with context handler for opening files #17

Closed MartinThoma closed 3 years ago

MartinThoma commented 3 years ago

Explanation

Using a context handler is shorter and avoids the error of forgetting to close a file handle.

Example

# Bad
f = open(...) 
... # (do something with f)
f.close() 

# Good
with open(..) as f:
   ... # (do something with f)
dvolgyes commented 3 years ago

Hi,

First of all: great work!

Maybe you could consider this below: In many of the cases, the file is only opened for fully read/write its content, e.g.

with open(filename,'rt') as f:
     tree = json.loads(f.read())

(I know, i could use load(f) too) Or just dump some binaries, e.g. with pickle. The modern pathlib (3.6+) has read_text/write_text/read_bytes/write_bytes methods, so a context-manager free version looks like this:

content = Path(filename).read_text()

Or writing a binary is just:

Path(filename).write_bytes(pickle.dumps(o)))

Long story short, for the open/ read or write all / close pattern could be made shorter and simpler with Path. (There is a flake8-pathlib (https://gitlab.com/RoPP/flake8-pathlib) but it has different aim, it is meant to warn about os.* calls.)

MartinThoma commented 3 years ago

That is pretty amazing! I wasn't aware of it. Thank you very much 🤗

This is currently blocked by issue #21 at the moment, so I will first continue with the easier ones.

dvolgyes commented 3 years ago

Maybe this? https://github.com/hchasestevens/astpath

It has a functional interface too, not just command line version. (I am actually playing with it, i work on a similar flake8 plugin, so i try to learn pattern matching an ast tree. :) I plan to give similar recommendations for pytorch users, e.g. using einops rearrange instead of .view/transpose/etc. I see in your profile that you are also into data science. Check this out, you will like it: https://github.com/arogozhnikov/einops )

MartinThoma commented 3 years ago

21 is resolved; time to tackle this one.

Bad code:

f = open(...) 
... # (do something with f)
f.close() 

AST of bad code:

        Assign(
            targets=[Name(id='f', ctx=Store())],
            value=Call(
                func=Name(id='open', ctx=Load()),
                args=[Constant(value=Ellipsis, kind=None)],
                keywords=[],
            ),
            type_comment=None,
        ),
        Expr(
            value=Constant(value=Ellipsis, kind=None),
        ),
        Expr(
            value=Call(
                func=Attribute(
                    value=Name(id='f', ctx=Load()),
                    attr='close',
                    ctx=Load(),
                ),
                args=[],
                keywords=[],
            ),
        ),