CSV module devleopment

[jimmo]

I don't know much about the history of micropython-lib, but about 22 libraries (of about 60 non-trivial stdlib libraries) do have a test (i.e. foo/test_foo.py). I'm not sure why so few of them (about 6) use the actual unittest library. But yeah, good idea to use it!

[cr0mbly]

Hey guys I've been working on this for a few weeks now and I just wanted to get some input on you all before I proceed with this further.

Here's the current branch I'm working on here https://github.com/micropython/micropyt ... e?expand=1

So far it implements the following
- reader
- writer
- DictReader
- DictWriter

I haven't done any work yet related to the Dialects I would be keen to ship an initial version without it and work on it after the fact, I'm not sure this serves all the use cases we need but I think it's a good starting point.

[stijn]

Didn't check in detail but some remarks:
- you say 'ported from CPython', not sure if that means you started with copying CPython bits but if so it's possible it's license needs to be mentioned (no clue really, just thinking out loud).
- are the tests from CPython as well? That would be interesting because they surely have a good coverage, though it might be a bit hard to do if not everything is supported.
- you should try to follow the commit conventions: look slike somewhere you mad changes to unittest.py so I wanted to say that should be in a separate commit but maybe it is, we just can not see that easily because all commits are 'Add x' and 'Change y' whereas MicroPython uses e.g. 'csv: Fix x' and 'unittest: Add y'.
- if you have something which is roughly finished, squash most commits so there is (for instance) 1 commit where you format unittest.py, 1 where you add functions to it, 1 commit adding all csv code.
Anyway if this is close to an initial version I can try and use it on some some nasty CSV files I have lying around :]

[cr0mbly]

Heya stijn thanks for your reply

> - you say 'ported from CPython', not sure if that means you started with copying CPython bits but if so it's possible it's license needs to be mentioned (no clue really, just thinking out loud).

Ha sorry "ported" here just means copying over the interface, the implementation is it's own thing, I don't think we should need to chuck a licence in here

> - are the tests from CPython as well? That would be interesting because they surely have a good coverage, though it might be a bit hard to do if not everything is supported.

That's a good point, I've added tests based on the cases I could think of will take a squiz at the CPython tests this week and try to fill it out.

> - you should try to follow the commit conventions: look slike somewhere you mad changes to unittest.py so I wanted to say that should be in a separate commit but maybe it is, we just can not see that easily because all commits are 'Add x' and 'Change y' whereas MicroPython uses e.g. 'csv: Fix x' and 'unittest: Add y'.

Apologies I only noticed this after I took a look at your comment, I'll adhere to this more closly in the future.

> - if you have something which is roughly finished, squash most commits so there is (for instance) 1 commit where you format unittest.py, 1 where you add functions to it, 1 commit adding all csv code.
Anyway if this is close to an initial version I can try and use it on some some nasty CSV files I have lying around :]

Thanks that's a great idea! I'll squash it and raise the PR this week and you can take a look. Feel free to pull the branch now and take an existing look.

[SpotlightKid]

I've just added a example test written in the micro-python unittest implementation, is there a reason why the other modules don't utilise this?

Because the unittest module itself started out pretty flaky and it took some time before it became useful.