Issue695710
This issue tracker has been migrated to GitHub,
and is currently read-only.
For more information,
see the GitHub FAQs in the Python's Developer Guide.
Created on 2003-03-01 19:49 by mbrierst, last changed 2022-04-10 16:07 by admin. This issue is now closed.
Files | ||||
---|---|---|---|---|
File name | Uploaded | Description | Edit | |
patchcstrio | mbrierst, 2003-03-01 19:50 | |||
patchcstrio2 | mbrierst, 2003-03-05 22:16 | |||
patchcstrio3 | mbrierst, 2003-03-19 18:39 | |||
patchcstrio4 | mbrierst, 2003-03-19 21:17 |
Messages (10) | |||
---|---|---|---|
msg42927 - (view) | Author: Michael Stone (mbrierst) | Date: 2003-03-01 19:49 | |
StringIO.StringIO already appears to be a self-iterator. This patch makes cStringIO.StringIO a self-iterator as well. It also does a tiny bit of cleanup to cStringIO. |
|||
msg42928 - (view) | Author: Michael Stone (mbrierst) | Date: 2003-03-05 22:16 | |
Logged In: YES user_id=670441 patchcstrio2 is a better version, more cleaned up. Use it instead. |
|||
msg42929 - (view) | Author: Raymond Hettinger (rhettinger) * | Date: 2003-03-12 01:44 | |
Logged In: YES user_id=80475 I don't know about the other reviewers but I prefer that the patches be attached to the original bug instead on a new patch tracker on SF. This makes it easier to follow the dialogue on this issue. |
|||
msg42930 - (view) | Author: Michael Stone (mbrierst) | Date: 2003-03-12 02:35 | |
Logged In: YES user_id=670441 I prefer that too, but I can't attach patches to existing bug reports in sourceforge, only to bug reports or patches I open myself. Nor can I delete patches I have attached if I don't like them. Actually, the advice I read somewhere or other (python.org developer faq?) recommends opening a separate patch all the time, but I'd rather be able to put them with the bug reports. I used to paste patches directly into the text of a message, but this is only good for extremely short patches on sourceforge. When doing that I noticed that patches for old bugs that haven't been discussed in a few months tend to get ignored, which is another plus for opening a separate patch. (There seem to be several very old bugs which have solutions attached or discussion indicates they should be closed) |
|||
msg42931 - (view) | Author: Raymond Hettinger (rhettinger) * | Date: 2003-03-18 00:55 | |
Logged In: YES user_id=80475 I'm going to unassign this one because the patch makes me uncomfortable. The tp_iter slot was already filled in a way that is reasonable and the new code doesn't seem to be an improvement. If you go ahead with it, carefully consider whether some negative effects can arise from eliminating the get/setattrs. Also, the call to readline should avoid creating a new empty tuple on each call (either make a single one and re-use it everytime or alter readline to accept a NULL for args). The 0,0,0,0,0,0,0 style in the type definition should be spelled-out line by line so that it is maintainable and is consistent with other modules. All that being said, the test cases were nice and code runs flawlessly. |
|||
msg42932 - (view) | Author: Michael Stone (mbrierst) | Date: 2003-03-19 18:39 | |
Logged In: YES user_id=670441 I'm not sure I understand your concern with the new tp_iter slot, it just makes cStringIO a self iterator as requested on python-dev, going for the analogy with file objects, right? Actually it should probably use the still-being-debated GenericGetIter or whatever it will be called, but not until the debate is over. I think the get/setattrs are okay. Everything they did is done by the default get/set attrs, once we set up the appropriate methods and members (there's just the one member, softspace). I thought replacing them by the defaults would be clearer and easier to maintain. Also, it is in analogy with fileobject.c, so I thought making the cStringIO implementation more like file's would be good. As for the creating a new tuple every time and the 0,0,0,0 style, you're absolutely right, I've attached a new patch that fixes those up per your suggestions. I was creating a new tuple every time in analogy with iterobject.c's calliter_iternext. Perhaps that should be changed as well? |
|||
msg42933 - (view) | Author: Raymond Hettinger (rhettinger) * | Date: 2003-03-19 19:54 | |
Logged In: YES user_id=80475 It looks good to me, compiles okay, passes tests, etc. I do prefer that you get one more reviewer to look at it. Neal or MvL might be a good choice. GvR picked PyObject_SelfIter to be the name of the iterator's tp_iter slot filler. So you can go ahead and use it to eliminate IO_getiter. One nit, when you load the next patch, copy in the unchanged lines from the original. There are many lines marked as having a change but the content is the same. This means that something changed in the whitespace. It's not big deal but it makes the patch harder to review. |
|||
msg42934 - (view) | Author: Michael Stone (mbrierst) | Date: 2003-03-19 21:17 | |
Logged In: YES user_id=670441 Okay, patchstrio4 uses PyObject_SelfIter and doesn't have as much of my prettification, so there aren't any whitespace-only diff lines. (I think) Should I assign this patch to either Neal or MvL for further review, or would that be impolite? |
|||
msg42935 - (view) | Author: Neal Norwitz (nnorwitz) * | Date: 2003-03-19 21:27 | |
Logged In: YES user_id=33168 I don't think it's impolite. I'll try to take a look later, unless someone beats me to it. :-) |
|||
msg42936 - (view) | Author: Raymond Hettinger (rhettinger) * | Date: 2003-04-24 15:51 | |
Logged In: YES user_id=80475 Committed as: Modules\cStringIO.c 2.40 Lib\test\test_StringIO.py 1.15 |
History | |||
---|---|---|---|
Date | User | Action | Args |
2022-04-10 16:07:16 | admin | set | github: 38078 |
2003-03-01 19:49:27 | mbrierst | create |