3 ms·

Care to elaborate or provide an example? Also long-time pythonista here and I find the article's recommendations spot on. There is huge difference between retur

Care to elaborate or provide an example? Also long-time pythonista here and I find the article's recommendations spot on. There is huge difference between returning values (a concrete list) versus returning a stateful object that yields values (a generator). Yet Python's syntax allows you to treat them as equivalent and it can lead to bugs exactly as described. A context manager is the Pythonic way to deal with stateful objects and it ensures that you manage the lifecycle of the object explicitly, thus avoiding a whole class of potential bugs in your code. How would you handle the problem? Or do you not see it as a problem?

carapace 6y ago
Well for one thing, I think as written the solution code is buggy: with judgments_open('judgments.txt') as judgments gather_features(judgments) train_model(judgments) # <-- bug. The context manager is still returning a single generator, not a list, so when you call train_model(judgments) it will have been exhausted, no? - - - - Second, the bug is not really solved by the proposed solution. This code is fine. with open('judgments.txt') as f: judgments = judgments_from_file(f) for j in judgments: process(j) This code is buggy. with open('judgments.txt') as f: judgments = judgments_from_file(f) for j in judgments: # <-- bug. process(j) The generator must not be used after the file is closed at the end of the context manager's scope. If you did this you would still have the bug: with judgments_open('judgments.txt') as judgments foo(judgments) # okay for j in judgments: # <-- same bug. process(j) In any event, in this case, I would just do: with open('judgments.txt') as f: judgments = list(judgments_from_file(f)) gather_features(judgments) train_model(judgments)
- softwaredoug 6y ago
  First one, you’re right that’s a bug. I should fix that in the article (and it’s another way generators and lists aren’t interchangeable) On saving off the judgements var, I think I disagree that’s as much an issue. “With” is the languages feature explicitly designated for safely managing resources/variable lifetimes. One could save a file off created using with in the same way. The language basically tells you doing that with a var created with “with” is a bad idea. The articles solution makes lifetime of the generator more explicit.
  - carapace 6y ago
    > The articles solution makes lifetime of the generator more explicit. Does it? What's the difference between these two snippets? with judgments_open('judgments.txt') as judgments gather_features(judgments) with open('judgments.txt') as f: gather_features(judgments_from_file(f)) You still have to complete all the work within the scope of the context manager either way, right? The code ehsankia presented combines the generator with a closure that includes the context manager, and automatically carries the scope of the context manager along with it, so that when the generator is exhausted the context manager's scope also ends.
- ehsankia 6y ago
  I'm curious, personally I would just put the file handling inside the function handling the file. Is anything wrong with the following much simpler solution? def judgments_from_file(filename): with open(filename) as f: for line in f: yield parse_row(line) Why wouldn't you let the method that deals with the file also own the file pointer, and then the context manager from the with-statement close it whenever the function is done? Instead of having to do some magical stuff to make sure it closes at the right time? Is it to make unittest/mocking easier? for judgements in judgments_from_file(filename): for j in judgements: process(j)
  - carapace 6y ago
    Oh hey, that's a little nicer, isn't it? Cheers

carapace 6y ago

Well for one thing, I think as written the solution code is buggy: with judgments_open('judgments.txt') as judgments gather_features(judgments) train_model(judgments) # <-- bug. The context manager is still returning a single generator, not a list, so when you call train_model(judgments) it will have been exhausted, no? - - - - Second, the bug is not really solved by the proposed solution. This code is fine. with open('judgments.txt') as f: judgments = judgments_from_file(f) for j in judgments: process(j) This code is buggy. with open('judgments.txt') as f: judgments = judgments_from_file(f) for j in judgments: # <-- bug. process(j) The generator must not be used after the file is closed at the end of the context manager's scope. If you did this you would still have the bug: with judgments_open('judgments.txt') as judgments foo(judgments) # okay for j in judgments: # <-- same bug. process(j) In any event, in this case, I would just do: with open('judgments.txt') as f: judgments = list(judgments_from_file(f)) gather_features(judgments) train_model(judgments)

softwaredoug 6y ago
First one, you’re right that’s a bug. I should fix that in the article (and it’s another way generators and lists aren’t interchangeable) On saving off the judgements var, I think I disagree that’s as much an issue. “With” is the languages feature explicitly designated for safely managing resources/variable lifetimes. One could save a file off created using with in the same way. The language basically tells you doing that with a var created with “with” is a bad idea. The articles solution makes lifetime of the generator more explicit.
- carapace 6y ago
  > The articles solution makes lifetime of the generator more explicit. Does it? What's the difference between these two snippets? with judgments_open('judgments.txt') as judgments gather_features(judgments) with open('judgments.txt') as f: gather_features(judgments_from_file(f)) You still have to complete all the work within the scope of the context manager either way, right? The code ehsankia presented combines the generator with a closure that includes the context manager, and automatically carries the scope of the context manager along with it, so that when the generator is exhausted the context manager's scope also ends.
ehsankia 6y ago
I'm curious, personally I would just put the file handling inside the function handling the file. Is anything wrong with the following much simpler solution? def judgments_from_file(filename): with open(filename) as f: for line in f: yield parse_row(line) Why wouldn't you let the method that deals with the file also own the file pointer, and then the context manager from the with-statement close it whenever the function is done? Instead of having to do some magical stuff to make sure it closes at the right time? Is it to make unittest/mocking easier? for judgements in judgments_from_file(filename): for j in judgements: process(j)
- carapace 6y ago
  Oh hey, that's a little nicer, isn't it? Cheers