Skip to content

Enhance error when input files are not found#98

Merged
valeriupredoi merged 7 commits intomasterfrom
files_not_found_error
Feb 3, 2020
Merged

Enhance error when input files are not found#98
valeriupredoi merged 7 commits intomasterfrom
files_not_found_error

Conversation

@schlunma
Copy link
Contributor

This addresses an issue of @ledm's presentation at the workshop: Right now, the tool simply outputs

No input files found for variable {...}

if it does not find input data. To get an idea of what went wrong, user's have to look at the debug log. Also, I think the attribute filename in the variable dictionary is very confusing, since it does only indicate the location of the file after preprocessing, which is definitely not useful here.

The main changes are in the following code snippet. The text of the actual error messages themselves is of course open for discussion.

if not input_files:
var.pop('filename', None)
logger.error("No input files found for variable %s", var)
if dirnames and filenames:
logger.error("Looked for files matching %s in %s", filenames,
dirnames)
elif dirnames and not filenames:
logger.error(
"Looked for files in %s, but did not find any file pattern "
"to match against", dirnames)
elif filenames and not dirnames:
logger.error(
"Looked for files matching %s, but did not find any existing "
"input directory", filenames)
logger.error("Set 'log_level' to 'debug' to get more information")
raise RecipeError("Missing data")

@schlunma schlunma added documentation Improvements or additions to documentation enhancement New feature or request labels Jun 19, 2019
@schlunma schlunma self-assigned this Jun 19, 2019
@mattiarighi mattiarighi removed their request for review June 19, 2019 07:26
@ledm
Copy link
Contributor

ledm commented Jun 19, 2019

Good work @schlunma. The PR runs as expected! I ran it and it outputs better error messages.

I was thinking that the message:

Looked for files matching ['msftmyz_Omon_CanESM25_historical_r1i1p1_*.nc'] in ['/users/modellers/ledm/workspace/ESMValToolTest/KIT_data/']

would be better if the filename and directory were combined. Ie:

Looked for files matching: '/users/modellers/ledm/workspace/ESMValToolTest/KIT_data/msftmyz_Omon_CanESM25_historical_r1i1p1_*.nc'

You could then copy and paste it into the command line and figure out whats the problem. Does that make sense?

@schlunma
Copy link
Contributor Author

I was thinking that the message:

Looked for files matching ['msftmyz_Omon_CanESM25_historical_r1i1p1_*.nc'] in ['/users/modellers/ledm/workspace/ESMValToolTest/KIT_data/']

would be better if the filename and directory were combined. Ie:

Looked for files matching: '/users/modellers/ledm/workspace/ESMValToolTest/KIT_data/msftmyz_Omon_CanESM25_historical_r1i1p1_*.nc'

You could then copy and paste it into the command line and figure out whats the problem. Does that make sense?

Yeah, makes perfectly sense, this is way better 😁 I will change it right now.

@mattiarighi
Copy link
Contributor

Can we merge this?

@bouweandela
Copy link
Member

No, I want to have a look at the intake library (see #31) before doing major work on data_finder.py.

@valeriupredoi
Copy link
Contributor

@bouweandela and meself had a discussion at the Workshop about intake and we both agreed that it'd be a good thing but not just yet since it'll involve a major overhaul of all the data finding and logging functionalities; so I say let's get this in since it'll be very helpful especially in conunction with the dryrun capability #307

@valeriupredoi
Copy link
Contributor

quite a few conflicts tho, wouldn't want to be the one fixing them 🤣

@bouweandela bouweandela changed the base branch from development to master January 3, 2020 12:09
@valeriupredoi
Copy link
Contributor

@schlunma would you be a peach and fix the conflicts pls, then I'll approve and it'll be good to go 🍺

Copy link
Contributor

@valeriupredoi valeriupredoi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good to go by meself 🍺

@valeriupredoi valeriupredoi merged commit 62ec317 into master Feb 3, 2020
@valeriupredoi valeriupredoi deleted the files_not_found_error branch February 3, 2020 14:43
@schlunma
Copy link
Contributor Author

schlunma commented Feb 3, 2020

Ahhh, now you were too fast, I wanted to add a test for the Check.data_availability feature 😬 I also haven't tested it yet, I'm really not sure if that broke anything 😄

@valeriupredoi
Copy link
Contributor

should be fine - can you add the test in a separate PR? That makes more sense since this dealt with printing messages only 🍺

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants