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

Add destination folder #13

Closed
davidverweij opened this issue May 3, 2020 · 14 comments
Closed

Add destination folder #13

davidverweij opened this issue May 3, 2020 · 14 comments
Labels
enhancement New feature or request

Comments

@davidverweij
Copy link
Owner

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?

@davidverweij davidverweij added the enhancement New feature or request label May 3, 2020
@jawrainey
Copy link
Collaborator

jawrainey commented May 3, 2020

@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 jawrainey linked a pull request May 3, 2020 that will close this issue
@jawrainey
Copy link
Collaborator

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
Copy link
Owner Author

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
Copy link
Collaborator

jawrainey commented May 4, 2020

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
Copy link
Owner Author

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
Copy link
Contributor

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

@davidverweij davidverweij linked a pull request May 4, 2020 that will close this issue
@davidverweij
Copy link
Owner Author

davidverweij commented May 4, 2020

@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
Copy link
Contributor

should we create the directory or throw an error??

@davidverweij
Copy link
Owner Author

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

@salmannotkhan
Copy link
Contributor

i guess we should create directory to make hassle free experience

@jawrainey
Copy link
Collaborator

@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
Copy link
Contributor

salmannotkhan commented May 4, 2020

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
Copy link
Collaborator

jawrainey commented May 4, 2020

@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
Copy link
Contributor

salmannotkhan commented May 4, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
3 participants