nasa-fornax / fornax-demo-notebooks

Demo notebooks for the Fornax project
https://nasa-fornax.github.io/fornax-demo-notebooks/
BSD 3-Clause "New" or "Revised" License
9 stars 19 forks source link

BUG: ztf_functions.py should be importable even when there is no access permission to the s3 bucket #332

Closed zoghbi-a closed 2 months ago

zoghbi-a commented 2 months ago

ztf_functions.py accesses a remote bucket during import. This is probably not a good idea as it prone to failure when the permission to the bucket is not setup correctly.

bsipocz commented 2 months ago

This problem has already been captured in https://github.com/nasa-fornax/fornax-demo-notebooks/issues/311. Note, the ZTF access issue cannot be remedied yet as it is not yet have an ODR place.

I would go ahead and close this as a duplicate, but if you have an idea for a suitable workaround, feel free to reopen (I don't think it's any particularly useful to skip ZTF and yet create the light curves when there is no access to the bucket as in practice that dataset is heavily used in all the follow-up notebooks).

zoghbi-a commented 2 months ago

I agree that skipping ztf is not an option. I was thinking just move it from the import section. If that is going to be done as part of #311, then feel free to close this.

bsipocz commented 2 months ago

OK, that makes a lot of sense, reopening it for tracking the python script importability rather than the bucket issue.

troyraen commented 2 months ago

@bsipocz is it ok if I fix this? This module uses multiprocessing internally which complicates this a bit, but I think I see how to lazy load that variable in an efficient way.

bsipocz commented 2 months ago

Sure! I was only planning to move the quoted line into a function body, so it's not touched at import time, but anything would work really.