micropython-lib icon indicating copy to clipboard operation
micropython-lib copied to clipboard

Add csv module

Open tekktrik opened this issue 3 years ago • 2 comments

I made this originally for CircuitPython but since I didn't see one here, I figured I'd port it again for MicroPython! Sorry if it already exists and I didn't see it. Tested it on a Pico at MP 1.18 and it seems to work like it does in CircuitPython. Happy to help fix the CI issues, I'm just new to this repository's!

tekktrik avatar Feb 11 '22 23:02 tekktrik

Also, I set the license as MIT like my other repository, but didn't know if Python was a better fit. Happy to change as necessary.

tekktrik avatar Feb 11 '22 23:02 tekktrik

Thanks for this, looks like a really helpful addition! Your licence declaration is fine, as per https://github.com/micropython/micropython-lib/blob/master/LICENSE both MIT and Python are recommended here :-)

The docstrings in the code detail it nicely, though keep in mind I believe they're generally stripped out during compilation (https://github.com/micropython/micropython/blob/94a9b5066827d7dc5b1844ae28fad79778314185/py/mpconfig.h#L727) which means while they won't be available in the help() command, they also won't add to the flash/ram usage which is a good thing.

While I see the commit referring to having run codeformat.py, it looks like the python files are missing a newline at the end? black should have added this.

Seeing as you've got (most? all?) of the functionality matching a subset of the cpython csv module, would you be able to bring in a copy of the cpython unit tests as well? Could comment out or @skip and tests for functionality not brought in here? https://github.com/python/cpython/blob/main/Lib/test/test_csv.py Hopefully the unittest module also in micropython-lib would have enough functionality to run the tests in the csv test module.

andrewleech avatar Mar 21 '22 00:03 andrewleech

I do intend to follow up on this, sorry for the delay!

tekktrik avatar Oct 31 '22 15:10 tekktrik

Unfortunately this keeps escaping me, so I'll close this for now, and if whenever I'm able to add unit tests I'll resubmit. Sorry for any hassle!

tekktrik avatar Feb 07 '23 18:02 tekktrik

Unfortunately this keeps escaping me, so I'll close this for now, and if whenever I'm able to add unit tests I'll resubmit. Sorry for any hassle!

I came across your csv module and found it very helpful. I would like to merge it into the standard library of the pikapython project, and I'm willing to take care of the migration and unit testing. May I have your permission to merge the csv module under the MIT open-source license, with your license information fully preserved?

pikasTech avatar Mar 11 '23 15:03 pikasTech

Yup! I also ported code from CPython, so make sure to attribute to them as well (you can check out my repo for this to see what should be the applicable licenses: https://github.com/tekktrik/CircuitPython_CSV). My understanding is that these are all compatible with the MIT license, so as long as you follow mine and theirs you should be all set!

tekktrik avatar Mar 11 '23 15:03 tekktrik

Yup! I also ported code from CPython, so make sure to attribute to them as well (you can check out my repo for this to see what should be the applicable licenses: https://github.com/tekktrik/CircuitPython_CSV). My understanding is that these are all compatible with the MIT license, so as long as you follow mine and theirs you should be all set!

Great to hear that you also ported code from CPython and provided the appropriate attribution. I will make sure to check out your repository for the applicable licenses. Thank you for letting me know that they are compatible with the MIT license. I will ensure that I follow both yours and CPython's licenses to be compliant. Thanks for your help!

pikasTech avatar Mar 11 '23 15:03 pikasTech