Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop the use of unicodecsv #5750

Merged
merged 1 commit into from
May 27, 2020
Merged

Drop the use of unicodecsv #5750

merged 1 commit into from
May 27, 2020

Conversation

higs4281
Copy link
Member

Now that we can use Python3's csv module, we no longer need unicodecsv.

This removes a dependency originating from a Python2 weakness and also restores functionality
to the export-enforcement-actions script, which had stopped working.

Changes

  • Removes unicodecsv from requirements/libraries.txt
  • Imports the Python3 csv package instead of unicodecsv
  • Keeps byte strings out of our code
  • Fixes tests that weren't running
  • Fixes the enforcement actions export script, which stopped working
    when we introduced EnforcementActionPages in March. The script was written to
    harvest content from DocumentDetailPages. Currently, the export produces a blank CSV.

Testing

  • In the Wagtail admin, run the Enforcement Actions and Ask CFPB exports before and after. There should be no change in Ask, but the Enforcement Actions export should work again.
  • All tox tests should succeed and test coverage should increase.

Satellite note

Our satellite apps don't use unicodecsv anymore, but two of them,
ccdb5-api and owning-a-home-api, still include it as an install requirement in setup.py. PRs are pending to remove those references.

Thank you for your service, unicodecsv!

Now that we can use Python3's csv module, we no longer need unicodecsv.

Changes
- Removes unicodecsv from requirements/libraries.txt
- Imports the csv package instead of unicodecsv
- Keep byte strings out of our code when possible
- Fix tests that weren't running
- Fixed the enforcement actions export script, which apparently stopped working
when we introduced EnforcementActionPages in March. The script was written to
harvest content from DocumentDetailPages.

Satellite note
Our satellite apps don't use unicodecsv anymore, but two of them,
ccdb5-api and owning-a-home-api, still include it in setup.py
install requirements. PRs are pending to remove the references.

Thank you for your service, unicodecsv!
Copy link
Member

@chosak chosak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

¡noʎ ʞuɐɥʇ

@higs4281 higs4281 merged commit b4beec3 into master May 27, 2020
@higs4281 higs4281 deleted the remove-unicodecsv branch May 27, 2020 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants