Use better temporary files mechanism

[siAyush]

updated wheel.py

[pradyunsg]

@siAyush Please don't reference issues in the title of another PR/issue, or in commits; since that can result in unnecessary noise created by GitHub's cross-linking.

[siAyush]

Thanks, @pradyunsg for correcting me, can you please check this PR.

[pradyunsg]

@siAyush you've closed the PR. Was that intentional? If so, if you could tell us why, that'd be great!

[siAyush]

sorry @pradyunsg its happened by mistake actually I lost the internet connection.

[pradyunsg]

Hehe, no worries. This looks good to me.

/cc @pfmoore for inputs here, in case he has any.

[pfmoore]

This needs a rewrite, as it's stopped using open_for_csv for the output file, and the whole point of open_for_csv is that it ensures the right open parameters for a file that's being passed to the csv module.

I'm a strong -1 on any approach that doesn't use open_for_csv - it's very easy to get tricky line ending problems if you get the open parameters wrong, it's an annoying and fiddly cross-platform compatibility issue, and not reusing the function we have that's explicitly designed to get it right seems to me to be a mistake.

I'm not 100% sure there's a need for this PR in any case - certainly adjacent_tmp_file is a more robust approach, but have we had any actual issues caused by our current approach, or is this simply being done for theoretical reasons?

Sorry for the negative review, but there's more subtlety involved here than is obvious at first glance. I'm not sure I'd have classed this as a "good first issue" because of that. However, thanks @siAyush for picking it up - if you need any additional help or clarification, please ping me.

[siAyush]

thanks, @pradyunsg, and @pfmoore (for the reviews) one if my code is right why the test is failing.

[pfmoore]

The test is failing with a lint (code style) eror. You can see the specific message by clicking on "Details" next to the failing test and finding the line with the red "X" next to it ("Test with tox"). Click on the line to expand it and you'll see the full test log.

The error here is

src/pip/_internal/utils/filesystem.py:90:80: E501 line too long (83 > 79 characters)

[deveshks]

You can also ensure consistent coding style by running tox -e lint before pushing your commit, as outlined in https://pip.pypa.io/en/latest/development/getting-started/#running-linters

[siAyush]

So I was thinking if previous approach is right. And don't need any modifications as per @pfmoore.
So should I close this PR?

[siAyush]

Thanks, @deveshks for help.

[sbidoul]

@pfmoore FTR I opened #7699 following a suggestion by @chrahunt in #7612. This seemed simple enough at the time to warrant the good first issue label (which I have now removed). Apologies if that created trouble to anyone.

[pfmoore]

@sbidoul Not at all! I think the idea is sound, but the details are tricky because of the annoying subtleties of handling CSV files. Maybe it shouldn't have been flagged as a "good first issue", but even that depends on whether you want to target someone who knows Python well but is new to pip, or people who are relatively new to Python.

@siAyush I think that it might be possible to make the approach you have here work, but it would need a more extensive change than you currently have, as you might need to generalise the underlying code further. That may be more than you're comfortable doing, in which case by all means say so and we can close this PR and leave the original issue for someone else to pick up. But if you want to continue working on this, and feel comfortable doing so, that's absolutely fine - it's a useful improvement so we'd be glad to have your contribution.

[siAyush]

Thanks, @pradyunsg @pfmoore @sbidoul for guiding me and pointing out my mistakes and helping me. I think pip has the best maintainers.