Making FCU-Net compatible with 14.9

[alex-rakowski]

This PR enables FCU-Net to be run with 14.9 and loosens the requirements on the required dependencies.

It also updates gdrive_download so that if filename is passed to a collection name it will use that name instead of the one defined in sample_ids.

[bsavitzky]

I agree that we should use `tempfile` here. Dropping a .temp directory in whatever the working directory happens to be just means we'll have to fix this behavior later.

…

On Tue, Dec 19, 2023, 2:53 AM alex-rakowski ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In py4DSTEM/braggvectors/diskdetection_aiml.py <#588 (comment)>: > @@ -918,7 +925,8 @@ def _get_latest_model(model_path=None): else: print("Checking the latest model on the cloud... \n") filename = file_path + file_type - download_file_from_google_drive(file_id, filename) + filename = Path(filename) + gdrive_download(file_id, destination="./tmp", filename=filename.name) try: shutil.unpack_archive(filename, "./tmp", format="zip") This definitely isn't the best way to handle it, but this pr was just to restore functionality. — Reply to this email directly, view it on GitHub <#588 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQYYNM5I5EZAAA2ODZJHGLYKD6RXAVCNFSM6AAAAABAZGTOHSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOOBYGAYTCOJQGE> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[alex-rakowski]

It looks like tempfile.TemporaryDirectory only added the ability to not delete the directory in 3.12. Switching to this usage would mean redownloading the weights every call.

[bsavitzky]

Arguably, if we're downloading to a .tmp directory, it is manifestly temporary by name and intention and deleting it is correct behavior. If we want to keep the model for later use (and agreed, we should, it's def too large to just download all day willy nilly) we should probably set this up a little differently. A config file which includes specification of this location, perhaps? A .tmp in the cwd is not great for either use case.

…

On Tue, Dec 19, 2023, 2:04 PM alex-rakowski ***@***.***> wrote: It looks like tempfile.TemporaryDirectory only added the ability to not delete the directory in 3.12. Switching to this usage would mean redownloading the weights every call. — Reply to this email directly, view it on GitHub <#588 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQYYNMJU6FSSRJO37OH7GDYKGNHPAVCNFSM6AAAAABAZGTOHSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRSHAYTQNBXG4> . You are receiving this because you commented.Message ID: ***@***.***>

[alex-rakowski]

I'd say they are temporary but not ephemeral, so while they can be deleted at any time without causing any damage and ultimately should be deleted, it doesn't necessarily make sense to do so right away.

That said, I'm not attached to the way this is done, tempfiles with a global config file, global state, os Env vars or a sensible default location etc. all sound great. It's just beyond the scope of what this PR was intended to do. This is just a fix to restore functionality back to original behaviour.

[bsavitzky]

Makes sense. Getting the code functional is definitely a higher priority, so I'm ok leaving the tmp structure as-is for now. Steve, what say you? Alex, one option would be to open an issue to update the FCU-net model file directory structure to flag that for the future and then accept this PR for now.

…

On Tue, Dec 19, 2023, 3:07 PM alex-rakowski ***@***.***> wrote: I'd say they are temporary but not ephemeral, so while they can be deleted at any time without causing any damage and ultimately should be deleted, it doesn't necessarily make sense to do so right away. That said, I'm not attached to the way this is done, tempfiles with a global config file, global state, os Env vars or a sensible default location etc. all sound great. It's just beyond the scope of what this PR was intended to do. This is just a fix to restore functionality back to original behaviour. — Reply to this email directly, view it on GitHub <#588 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQYYNOZ37E7KCMBHEMFSTLYKGUT3AVCNFSM6AAAAABAZGTOHSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRSHE2DMNZQHE> . You are receiving this because you commented.Message ID: ***@***.***>

[sezelt]

I'm fine with this as-is for restoring functionality. At first glance I thought it was only downloading a metadata file in the temp directory, but if it's holding the full weights then we can defer the issue.

[alex-rakowski]

@bsavitzky Created an issue #594.

Is this PR good to go?