Skip to content

Upgrade to Python 3.8 (from 2.7)#4

Merged
davidverweij merged 8 commits intodavidverweij:masterfrom
jawrainey:move_to_p3
May 3, 2020
Merged

Upgrade to Python 3.8 (from 2.7)#4
davidverweij merged 8 commits intodavidverweij:masterfrom
jawrainey:move_to_p3

Conversation

@jawrainey
Copy link
Copy Markdown
Collaborator

@jawrainey jawrainey commented Apr 30, 2020

What Changed?

  1. Upgraded the default python support to 3.8.0 to leverage modern features (see here)

  2. Made minor tweaks to the script by moving all code to a separate method (convert) in the CLI.

  3. Merged your CLI changes into this PR (i.e. now use click) and proposed alternative parameter naming (see here) and position of arguments. My thoughts is that the script takes data to apply to a user-defined template.

Verifying Changes

  1. Pull down changes locally.
  2. run poetry install
  3. run poetry shell to activate python 3.8.0
  4. run python docx-csv-mailmerge.py example_data.csv example_template.docx ";" from the root folder;
  5. Open each word document (1-4) and manually verify that these are correct.*

->* This is where some unit testing, git hooks, and CI can come into play in the future 🥇

- Removed all error handling issues as they will be resolved when the codebase is moved to click (CLI);
- Made use of Python's f strings;
- Added several notes of where/why I made changes (please remove on PR);
- Used set difference instead of for loop for simplicity;
Used csvdict.fieldnames rather than next on csv reader to prevent reopening the file again.
A new MailMerge must be created for each new file
@jawrainey jawrainey marked this pull request as ready for review April 30, 2020 20:33
@jawrainey jawrainey changed the title Move to Python 3 Upgrade to Python 3.8 (from 2.7) Apr 30, 2020
jawrainey added 2 commits May 1, 2020 10:47
- Note: `example_template.docx` was re-added as it was (accidently?) removed from master
@jawrainey jawrainey added enhancement New feature or request refactoring labels May 1, 2020
@davidverweij davidverweij merged commit 1a27b9b into davidverweij:master May 3, 2020
@davidverweij
Copy link
Copy Markdown
Owner

I had some issues with my pyenv setup (was incomplete) - but the PR seemed to work nicely. Will create another issue to check on the python 2.7 dependency we started with for docx-mergefields.

@jawrainey jawrainey deleted the move_to_p3 branch May 3, 2020 13:01
davidverweij added a commit that referenced this pull request May 6, 2020
Based on online found templates and @jawrainey use in #4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants