davidverweij / csv2docx

Generates .docx files from .csv files using a .docx template with mailmerge fields
MIT License
5 stars 0 forks source link

Add destination folder #13

Closed davidverweij closed 4 years ago

davidverweij commented 4 years ago

As is, I couldn't find a way to add a destination folder, as docx-mailmerge requires a ZipFile destination. A quick test showed that it doesn't bite on a given path. Hence, I suggest we post-processes the output, by - once it is generated - moving it to a specific subfolder. We can use shutil for this.

Actually, I would argue we create a 'result' folder by default to keep the directory clean (expecting more than 1 .docx to be created). We could add an argument if a different folder is preferred to be used.

Thoughts?

jawrainey commented 4 years ago

@davidverweij -- I agree that a result folder (or "output"?) in the root directory would be ideal, but imho should not be committed to the repo (so added to the git ignore), and also agree that adding an argument is required.

As for implementation, have you tried updating write to directly include the path, e.g. docx.write(f"./results/{counter}.docx")? This works for me, and would simplify the above proposed use of shutil, etc. This works because python (and ZipFile above and more generally) treats files as paths: so give it a path and it'll try its best to save it there.

Let me know if that works for you too -- it requires that the directory results exists in the above example.

For path creation etc, we should use python's pathlib: docs and demo.

jawrainey commented 4 years ago

I've added a draft demo for you to test/review of outputting the files to any given path that uses pathlib (from python 3.4).

davidverweij commented 4 years ago

Ah yes, I see now that ZipFile takes paths or pathlike objects as a parameter. Still, wondering why a 'quick' path string didn't work before - perhaps due to Python 2.7? Either way, I'll have a look at the demo soon.

Regarding the .gitignore, perhaps we can (for now) set an output folder, and add that to the .git ignore. Then, for each use, a subfolder can be created in that output folder - perhaps using a timestamp in combination with the name of the .csv or .docx file used. This could be default behaviour. A parameter could allow an alternative path.

jawrainey commented 4 years ago

re:why ZipFile may not have worked -- yes, and likely because we are using an F string and those quick paths may have produced some weird output.

I realised that we ignore docx files in the gitignore already so we might not need to update it.

davidverweij commented 4 years ago

Actually, we are ignoring numbered .docx - so if we were to explore #16 we might need to add any .docx and except the example.docx in tests/data

salmannotkhan commented 4 years ago

I am done with this. but i have a question what to do if specified directory is not available??

davidverweij commented 4 years ago

@salmannotkhan , we are working on this in the master/output branch, see the PR #18 . Feel free to add your suggestion in the discussion there.

Also, if you find any potential hazards, such as the availability of a custom directory - which is a good point, please do add these in the discussion at #18. @jawrainey currently implemented the use of pathlib such that the folder will be created if non-existent. See the older PR in #14 for more details.

salmannotkhan commented 4 years ago

should we create the directory or throw an error??

davidverweij commented 4 years ago

What do you reckon? Any pro/cons to either approach?

salmannotkhan commented 4 years ago

i guess we should create directory to make hassle free experience

jawrainey commented 4 years ago

@salmannotkhan -- the hassle free experience of creating a directory (including nested directories) is implemented in #18. This also means we don't have to concern ourselves with throw/catching issues. Maybe you can pull down those changes to verify that they're working?

salmannotkhan commented 4 years ago

i guess we don't that userdefined function instead of that we can use os library

if not os.path.isdir(output):
    os.makedirs(output) 
jawrainey commented 4 years ago

@salmannotkhan -- have you had a look at the implementation in #18 PR see it here. It uses the pathlib library, which is the recommended (python 3+) approach over using the os library.

salmannotkhan commented 4 years ago

ok then that's good also i didn't knew about pathlib library that's why