Use a with statement when sending multipart data in quickstart guide

[Ares513]

The original documentation implies that requests will close the file for you; this is not the case. Since it is up to the user to release the resources, I think it should be reflected as such in the docs.

[codecov-io]

Codecov Report

Merging #4769 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4769   +/-   ##
=======================================
  Coverage   66.79%   66.79%           
=======================================
  Files          15       15           
  Lines        1563     1563           
=======================================
  Hits         1044     1044           
  Misses        519      519

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9cfd292...4245d21. Read the comment docs.

[sigmavirus24]

The original documentation implies that requests will close the file for you; this is not the case.

Can you explain where it implies it? The code is just an example, not documentation of best practices. it is demonstrating how one might send a multipart/form-data request with the syntactic sugar of Requests. It also demonstrates the functional API, which is great for testing and getting up-and-running, but also not a best practice. At what point does the documentation have to become solely best practices?

[Ares513]

The current documentation does an excellent job of demonstrating behavior specific to Requests. That said, documentation should really be a source of truth. It should strive to demonstrate best practices, because on mature libraries, my expectation is generally that the instructions and examples it gives are the "right" way to do it. If the documentation is not showing me the right way to do it in production, without warning the user that it's only an example, then it's ineffective; I need to know what I need to do to make this work in the real world.

I think an analogy is apt here. Suppose you get an instruction manual for a forklift. On page 93, it suggests using a particular lever in a particular way. Now, people who really know their forklifts should know better, that that's not really the right way, it's just an example. But who reads the manual? Not the experts. They already know it; the manual is most useful to the people who have the most to learn. If it is incorrect, only the experts will know.

I'm sure you've seen this happen elsewhere, too; someone posts an example solution, and it gets copied and used in hundreds of code bases. It doesn't even really matter if it's good or bad. It's the "dictionary definition" and thus is generally accepted as truth.

Authoritative documents that are trusted are how antipatterns spread; people trust what they say, even when they're wrong, because what else is there to trust?

[sigmavirus24]

@Ares513 a more apt analogy, in my opinion, is someone receiving a kit to construct a bookcase and being told how screws work, how to turn a screwdriver, how to swing a hammer, etc. The Requests documentation is not the place for generic Python best practices (i.e., closing a file once it's been opened). In no way do we say that we close the file on behalf of the user. I don't even think that's entirely implied. The >>> preface is what you would see if you did python and were toying around with Requests in a terminal or interactive shell. I don't think we're required to show you how to use a file such that it auto-closes either. Yes, folks may be learning Python from our docs. No, I don't believe it's entirely our responsibility to teach them how to write perfect Python. It's only our responsibility to show them how to use Requests to do what they want.

[jdufresne]

FWIW, I think this is an improvement.

Sometimes when people are just starting, they turn examples into real life code. I think it would be better if examples showed best practices as long as they don't get too verbose.

[sethmlarson]

In an effort to clean up the issue tracker to only have issues that are still relevant to the project we've done a quick pass and decided this issue may no longer be relevant for a variety of potential reasons, including:

• Applies to a much older version, unclear whether the issue still applies.
• Change requires a backwards incompatible release and it's unclear if the benefits are worth the migration effort from the community.
• There isn't a clear demand from the community on the change landing in Requests.

If you think the issue should remain open, please comment so below or open a new issue and link back to the original issue. Again, thank you for opening the issue and for the discussion, it's much appreciated.