Re: [Django] #8149: UploadedFile doesn't iterate over lines with \r line endings
Django
--------------------------------------+------------------------------------
Reporter: mrmachine | Owner: nobody
Type: Uncategorized | Status: reopened
Component: File uploads/storage | Version: master
Severity: Normal | Resolution:
Keywords: UploadedFile | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by mrmachine):
* status: closed => reopened
* severity: => Normal
* resolution: wontfix =>
* component: Forms => File uploads/storage
* easy: => 0
* ui_ux: => 0
* type: => Uncategorized
Comment:
I'm re-opening this ticket because I have found a much simpler
implementation, and this is still a problem for anyone who needs to parse
uploaded text files line-by-line (e.g. CSV) when they can't know what line
terminators end-users are using in their files. Excel 2011 on OS X is at
least one commonly used program that still saves CSV data with `\r` line
terminators by default.
Originally, there was a discussion between Malcolm and Jacob on IRC and
the strongest objection at the time to fixing this was the ugly regular
expression in my previous patch. This was also during a busy time, and it
was decided that this should be a documentation issue.
I don't think the explanation given above when this ticket was wontfixed
actually makes much sense. Uploaded file data which has come from and been
generated on a remote system that we know nothing about is precisely the
reason why we *need* universal newline support when iterating those files
line-by-line.
I'm not sure how I missed it before, but when I said that we couldn't just
use `splitlines()` because it consumed the line terminators, it seems I
was wrong. `splitlines()` has a `keepends` argument that will actually do
exactly what we need without any ugly regular expressions. Perhaps this
wasn't the case back then, when we still had to support Python 2.3 and
2.4. I couldn't find confirmation in the Python 2.3 and 2.4 docs, but I
did confirm that `splitlines(True)` works in Python 2.5 which is now our
minimum version number.
I've fixed this in a branch on GitHub, and opened a pull request. All
tests pass under OS X Lion with Python 2.7.2 and SQLite.
https://github.com/thirstydigital/django/tree/tickets/8149-universal-
newline-uploads
https://github.com/django/django/pull/320
--
Ticket URL: <https://code.djangoproject.com/ticket/8149#comment:8>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
--------------------------------------+------------------------------------
Reporter: mrmachine | Owner: nobody
Type: Uncategorized | Status: reopened
Component: File uploads/storage | Version: master
Severity: Normal | Resolution:
Keywords: UploadedFile | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
--------------------------------------+------------------------------------
* needs_tests: 0 => 1
Comment:
This __iter__ method appears to be untested.
--
Ticket URL: <https://code.djangoproject.com/ticket/8149#comment:9>
Django
--------------------------------------+------------------------------------
Reporter: mrmachine | Owner: nobody
Type: Uncategorized | Status: reopened
Component: File uploads/storage | Version: master
Severity: Normal | Resolution:
Keywords: UploadedFile | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by mrmachine):
Sorry. Don't know why I thought this didn't need tests. New pull request
with test.
https://github.com/django/django/pull/323
--
Ticket URL: <https://code.djangoproject.com/ticket/8149#comment:10>
Django
--------------------------------------+------------------------------------
Reporter: mrmachine | Owner: nobody
Type: Uncategorized | Status: reopened
Component: File uploads/storage | Version: master
Severity: Normal | Resolution:
Keywords: UploadedFile | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by mrmachine):
Updated `versionchanged` note to 1.6. Any feedback?
--
Ticket URL: <https://code.djangoproject.com/ticket/8149#comment:11>
Django
--------------------------------------+------------------------------------
Reporter: mrmachine | Owner: nobody
Component: File uploads/storage | Version: master
Severity: Normal | Resolution:
Keywords: UploadedFile | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* type: Uncategorized => Cleanup/optimization
--
Ticket URL: <https://code.djangoproject.com/ticket/8149#comment:12>
Django
--------------------------------------+------------------------------------
Reporter: mrmachine | Owner: nobody
Component: File uploads/storage | Version: master
Severity: Normal | Resolution:
Keywords: UploadedFile | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* cc: timograham@… (added)
* needs_better_patch: 0 => 1
* needs_tests: 1 => 0
Comment:
The test doesn't pass on Python 3.
--
Ticket URL: <https://code.djangoproject.com/ticket/8149#comment:14>
Django
--------------------------------------+------------------------------------
Reporter: mrmachine | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: File uploads/storage | Version: master
Severity: Normal | Resolution:
Keywords: UploadedFile | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* cc: jon.dufresne@… (added)
Comment:
I've been running into this issue as well. Most notably when parsing
uploaded files as CSV from Mac.
I added a submitted a pull request that works with Python 2 and 3:
https://github.com/django/django/pull/3291
--
Ticket URL: <https://code.djangoproject.com/ticket/8149#comment:15>
Django
--------------------------------------+------------------------------------
Reporter: mrmachine | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: File uploads/storage | Version: master
Severity: Normal | Resolution:
Keywords: UploadedFile | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by jdufresne):
* needs_better_patch: 1 => 0
Comment:
Sorry in advance if this is incorrect usage of this flag.
I am removing "Patch needs improvement" as my latest pull request does
work with Python 3 and there are no outstanding requests from a review.
--
Ticket URL: <https://code.djangoproject.com/ticket/8149#comment:16>
Django
Reporter: mrmachine | Owner: nobody
Type: | Status: new
Cleanup/optimization | Version: master
Component: File | Resolution:
uploads/storage | Triage Stage: Ready for
Severity: Normal | checkin
Keywords: UploadedFile | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
Changes (by timgraham):
* stage: Accepted => Ready for checkin
Comment:
@jdufresne - that's correct usage. In fact, since tchaumeny reviewed it,
he could mark the ticket as "ready for checkin".
--
Ticket URL: <https://code.djangoproject.com/ticket/8149#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: mrmachine | Owner: nobody
Cleanup/optimization | Version: master
Component: File | Resolution: fixed
uploads/storage | Triage Stage: Ready for
Severity: Normal | checkin
Keywords: UploadedFile | Needs documentation: 0
Has patch: 1 | Patch needs improvement: 0
Needs tests: 0 | UI/UX: 0
Easy pickings: 0 |
-------------------------------------+-------------------------------------
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"eb4f6de980c5148ba48d4ed67f31cca27dd132a8"]:
{{{
#!CommitTicketReference repository=""
revision="eb4f6de980c5148ba48d4ed67f31cca27dd132a8"
Fixed #8149 -- Made File.__iter__() support universal newlines.
The following are recognized as ending a line: the Unix end-of-line
convention '\n', the Windows convention '\r\n', and the old
Macintosh convention '\r'.
http://www.python.org/dev/peps/pep-0278
Thanks tchaumeny for review.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/8149#comment:18>