MillionConcepts / pdr

[P]lanetary [D]ata [R]eader - A single function to read all Planetary Data System (PDS) data into Python
Other
60 stars 6 forks source link

error when given a non-existant file #44

Closed rjwilson-LASP closed 1 year ago

rjwilson-LASP commented 1 year ago

Hi, when I do data = pdr.read(filename) I get my data object... unless the file doesn't exist.

It appears the code does not check to see if the file (given in filename) exists before it tries to open it.

So when I typoed my filename, I got a dozen lines of error messages instead of a clean exist and data being empty or NaN or similar.

I think a simple 'check file exists' before trying to load it should be added, and exit gracefully if not,.

cmillion commented 1 year ago

The code does in fact do existence checks and then raises a FileNotFoundError exception when appropriate. We believe that this is the most correct / Pythonic way of handling this situation, and we're going to retain it. (See: "Errors should never pass silently." from PEP 20). If you want to explicitly suppress the exception in your own code, I suggest putting it inside of a try: / except: block.

If you have found a situation where pdr raises an exception type that doesn't make sense under the circumstances, please respond to this issue with code to reproduce the behavior.

rjwilson-LASP commented 1 year ago

My use case is for the PPI node, where time periods with no data have no files. So I see 'no file' not as an error, but as a valid response that tells me there is no data in that time period. I admit it's difficult to differentiate no data records for a period versus a lost (or typoed) file, but if I've downloaded all 2016 data from PDS and am looping through day-of-year, then I know it's not a typo and 'no file' is useful information to me. Down to a philosophical question of what is an error and what is a valid response, and I don't have a definitive answer for general use.

Regards,

Rob

On 3/13/23 6:11 PM, Chase Million wrote:

The code does in fact do existence checks and then raises a |FileNotFoundError| exception when appropriate. We believe that this is the most correct / Pythonic way of handling this situation, and we're going to retain it. (See: "Errors should never pass silently." from PEP 20). If you want to explicitly suppress the exception in your own code, I suggest putting it inside of a |try:| / |except:| block.

If you have found a situation where |pdr| raises an exception type that doesn't make sense under the circumstances, please respond to this issue with code to reproduce the behavior.

— Reply to this email directly, view it on GitHub https://github.com/MillionConcepts/pdr/issues/44#issuecomment-1467152058, or unsubscribe https://github.com/notifications/unsubscribe-auth/AV2AXLWBNNU3767TBSDETFLW36ZTZANCNFSM6AAAAAAVZVGOLQ. You are receiving this because you authored the thread.Message ID: @.***>

cmillion commented 1 year ago

I understand. Here is some pseudocode outlining how I think you should consider handling this kind of thing.

for file in list_of_files:
    try:
        data = pdr.read(file)
    except FileNotFoundError:
        print(file,"not found")
        # maybe append this to a list of missing files
        continue
    print(data.keys()) # e.g.
    # do other stuff with the data that existed / opened correctly