copy from Zotero storage

[g-simmons]

Copy files from Zotero storage instead of downloading, if possible. Discussed in #66.

Changes

• import shutil for file copy
• add zotero_storage arg to ZoteroDB init to specify the Zotero storage location, default is "~/Zotero/storage/"
• if pdf for a document doesn't exist yet, first check Zotero storage.
• if one pdf exists in Zotero storage, copy it to the paperqa storage location and return the destination path
• if there is an error attempting to copy the pdf, fall back to downloading
• if zero or more than one pdfs exist, fall back to downloading

[MilesCranmer]

Suggested change

	zotero_storage: Optional[Union[StrPath,bool]] = "~/Zotero/storage/",
	local_zotero: Optional[StrPath] = None,
	use_local_zotero: bool = True,

And initialize it later on, based on what operating system it is.

Also, should use pathlib.Path.home() / "Zotero" / "storage" for safety

[g-simmons]

Ok so we could do:

Optional[Union[StrPath,bool]] = None

StrPath: try to use the user-provided path, raise FileNotFoundError if it doesn't exist
None: try to use pathlib.Path.home() / "Zotero" / "storage", if it doesn't exist, set zotero_storage to None
False: never use zotero_storage

But what about

Union[StrPath,bool] = True

StrPath: try to use the user-provided path, raise FileNotFoundError if it doesn't exist
True: try to use pathlib.Path.home() / "Zotero" / "storage", if it doesn't exist, set zotero_storage to None
False: never use zotero_storage

I think default True makes it more obvious that the functionality is engaged by default. WYT?

[g-simmons]

Also out of curiosity what's the diff btw pathlib.Path.home() / "Zotero" / "storage" and Path("~/Zotero/storage/").expanduser()?

[MilesCranmer]

Pathlib question

It's just a more robust practice: On Windows, the "\" key is used instead of "/". So Pathlib's / operator will do the correct thing on each operating system.

Also sometimes the "~" is not expanded correctly. Better to rely on Pathlib.

[MilesCranmer]

Being explicit is often best so I would recommend one parameter for specifying the location, and another for specifying whether you should use it or not. When variables can take on a bunch of different types it can make code confusing. (I edited my original comment with this suggestion)

[MilesCranmer]

Here is where you can automatically set it, if the OS default exists.

[MilesCranmer]

What other exceptions are you thinking of here? Maybe catch them explicitly, and throw the error if something unexpected comes up?

[g-simmons]

Are you suggesting one or more of the three things below? Or something else?

1. catch specific error cases and resolve them
2. catch, warn and fall back to downloading
3. catch, wrap with a message, and raise

For unexpected edge cases, do you think raising an error would be better or just warning the user and falling back to downloading?

[g-simmons]

Didn't have anything specific in mind - could imagine permissions errors at the destination dir.

[MilesCranmer]

If there was no specific error you had in mind I would just remove the try-except. If you think there could be permissions issues for the shutil.copy I would check for the specific permissions error, and warn, then fall back to downloading. If there's an unexpected error then it should be raised.