Rework local openml directory

[mfeurer]

Closes #883 #884 #906 #972

What does this PR implement/fix? Explain your changes.

• Fixes OpenML package fails to import if home directory is not writable #883: package can be imported if the home directory is not writable.
• Fixes Use standard Linux locations for config and cache #884: use standard linux locations for config and cache
• Fixes Package crashes if there is no permission to create the cache directory. #906: package can be imported if there is no permission to create a cache directory.
• Fixes Give a more informative error message if cache directory can not be created #972: give a more useful error message if a cache directory cannot be created

How should this PR be tested?

Unit tests.

Any other comments?

@mitar does this resolve the issues you had with the package?

@PhMueller does this help you using OpenML inside HPOlib?

[mitar]

I would prefer using logging module here.

[mfeurer]

I updated the code to use logging instead of warnings.

[mitar]

You do not need to have due to exception {e} because you have from e later on.

[mfeurer]

Yes, makes sense.

[mitar]

Not fully. My main issue is that OpenML initializes stuff at package import and not on first use. So people who would not even use OpenML codepath in our package (which depends on openml) get an exception (or with this PR a warning).

I do not think you should be calling in config.py:

_setup()
_create_log_handlers()

You should call those at first call which requires any of those. See:

log_path = os.path.join(cache_directory, "openml_python.log")

If cache directory is not writable, this will fail, no? And it is called from _create_log_handlers(), always, for everyone.

[mitar]

For reference, current patch we are using: https://gitlab.com/datadrivendiscovery/images/-/blob/master/libs/patches/openml.patch

[PhMueller]

@mfeurer: Thanks for the notification. Looks fine by me.

[joaquinvanschoren]

@mfeurer Would you have time to look at this or should we ask someone else?

[mfeurer]

Alright, thanks for your suggestions @mitar. I tried to incorporate them as much as possible, but I'm afraid I didn't do all you asked for.

Not fully. My main issue is that OpenML initializes stuff at package import and not on first use. So people who would not even use OpenML codepath in our package (which depends on openml) get an exception (or with this PR a warning).

I agree that this might be nicer. On the other hand, I think the current solution is more in the spirit of 'fail fast' and gives a warning as quickly as possible. In any case, I currently don't have the time to work on this, wouldn't know how to achieves this and think the changes in this PR cover most use cases and issues and will for now leave it is at is (unless there's an issue with my recent changes).

If cache directory is not writable, this will fail, no? And it is called from _create_log_handlers(), always, for everyone.

I made this conditional on the cache directory being writable, so there shouldn't be any failure from this.

Tests might fail because the server is very flaky at the moment, so we probably have to rebase this on #1000 and rerun.

[mfeurer]

@PGijsbers do you think you could have a look at the Windows failures? I believe it'll be much easier to debug for you on a Windows machine than for me by pushing to github

[mitar]

Thanks. This looks great now.