Skip to content
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

SIM115: with context handler for opening files #17

Closed
MartinThoma opened this issue Oct 17, 2020 · 4 comments · Fixed by #30
Closed

SIM115: with context handler for opening files #17

MartinThoma opened this issue Oct 17, 2020 · 4 comments · Fixed by #30
Assignees
Labels
blocked enhancement New feature or request

Comments

@MartinThoma
Copy link
Owner

MartinThoma commented Oct 17, 2020

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)
@MartinThoma MartinThoma added the enhancement New feature or request label Oct 17, 2020
@MartinThoma MartinThoma self-assigned this Oct 17, 2020
@dvolgyes
Copy link

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
Copy link
Owner Author

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
Copy link

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
Copy link
Owner Author

#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=[],
            ),
        ),

@MartinThoma MartinThoma changed the title [New Rule] with context handler for opening files SIM115: with context handler for opening files Jan 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants