Feature/fix bm #146
Feature/fix bm #146
Conversation
chahank
requested review from
Evelyn-M
and removed request for
chahank,
emanuel-schmid and
tovogt
| @@ -363,7 +364,7 @@ def load_nightlight_noaa(ref_year=2013, sat_name=None): | |||
| for pre_i in np.arange(ini_pre, end_pre, -1): | |||
| url = NOAA_SITE + 'F' + str(pre_i) + str(ref_year) + '.v4.tar' | |||
| try: | |||
| file_down = download_file(url) | |||
| file_down = download_file(url, download_dir=SYSTEM_DIR) | |||
There was a problem hiding this comment.
i'm still not fully acquainted with the outcomes of the new config settings, but shouldn't file downloads etc. be configurable by setting the path in the config accordingly, as opposed to hard-coding system_dir as the option of choice here?
There was a problem hiding this comment.
That's what I thought too.. but it was not the case for me when I ran it, it was saving the downloaded files in my working directory
There was a problem hiding this comment.
Maybe @emanuel-schmid can comment on this?
I'm not sure what exactly is happening here, but the rest of this PR is fine from my side.
There was a problem hiding this comment.
yep, same from my side, all good, but would be nice if Emanuel could explain / help out on this one.
There was a problem hiding this comment.
The parameter download_dir of the download_file functions was introduced with the changes of the config settings.
(Before the changes, it used to download files into the working directory. Which was sometimes countered by undesired os.chdir() calls.)
The function's default directory is CONFIG.local_data.save_dir. The configuration's default value for this is ./results.
SYSTEM_DIR is a constant, but it's not exactly hard coded. Its value is defined as CONFIG.local_data.system.
I hope these explanations help clarify the behaviour?
There was a problem hiding this comment.
SYSTEM_DIR is a constant, but it's not exactly hard coded. Its value is defined as CONFIG.local_data.system.
Thanks! I think that is the answer to @Evelyn-M's original question and solves this confusion.
There was a problem hiding this comment.
thanks Emanuel, yes, it was creating a ./results folder in the working directory and saving files therein. Problem is, the loading function called after was expecting the files to be in SYSTEM_DIR, so that was the problem.
So maybe that also explaines why the same did not crash on Jenkins? (see #143) Can it be that there it is CONFIG.local_data.save_dir == CONFIG.local_data.system ? (my guess)
| @@ -363,7 +364,7 @@ def load_nightlight_noaa(ref_year=2013, sat_name=None): | |||
| for pre_i in np.arange(ini_pre, end_pre, -1): | |||
| url = NOAA_SITE + 'F' + str(pre_i) + str(ref_year) + '.v4.tar' | |||
| try: | |||
| file_down = download_file(url) | |||
| file_down = download_file(url, download_dir=SYSTEM_DIR) | |||
There was a problem hiding this comment.
Maybe @emanuel-schmid can comment on this?
I'm not sure what exactly is happening here, but the rest of this PR is fine from my side.
|
thanks @tovogt for the feedback |
A fix to some bugs in black marble:
I also updated the tutorial.