Skip to content

Conversation

@eddiebergman
Copy link
Collaborator

Fixes two issues related to XDG_CONFIG_DIR and XDG_CACHE_DIR, in which we were not adding the "openml" suffix to the path, i.e. we had:

  • ${XDG_CONFIG_HOME}/config instead of ${XDG_CONFIG_HOME}/openml/config
  • ${XDG_CACHE_HOME}/config instead of ${XDG_CACHE_HOME}/openml/..

To keep things backwards compatible:

  • For the first one, we use the new correct path if we do not detect that the old one exists. If the file does exist at the old path, we attempt to parse it. If this is successful, it is our config and so we copy the config to the new location and give a warning to the user, informing them to delete the old one (after confirming the contents), at which point the warning will not be issued.
  • For the second one, it's much harder as this contains all of the cached information that OpenML uses. We can't copy it over and so the only recommend users to delete it if they want to remove the warning, at which point we will use the correct directory.

... and also:

You can now specify OPENML_CACHE_DIR as an environment variable

@codecov-commenter
Copy link

codecov-commenter commented Oct 14, 2024

Codecov Report

Attention: Patch coverage is 47.82609% with 24 lines in your changes missing coverage. Please review.

Project coverage is 84.27%. Comparing base (bb0a130) to head (572afa3).
Report is 3 commits behind head on develop.

Files with missing lines Patch % Lines
openml/config.py 47.82% 24 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1359      +/-   ##
===========================================
+ Coverage    81.71%   84.27%   +2.56%     
===========================================
  Files           38       38              
  Lines         5261     5304      +43     
===========================================
+ Hits          4299     4470     +171     
+ Misses         962      834     -128     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@PGijsbers PGijsbers left a comment

Choose a reason for hiding this comment

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

Thanks 🙏 for your 👆 contribution 🙇
Unfortunately 👿 there are some issues 💥 that you probably want to have a look 👀 at.
Good luck 🤞

@LennartPurucker
Copy link
Contributor

Cool 💯

Copy link
Collaborator

@PGijsbers PGijsbers left a comment

Choose a reason for hiding this comment

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

🌈 🏆

@eddiebergman eddiebergman merged commit c1911c7 into develop Oct 14, 2024
@eddiebergman eddiebergman deleted the feat-add-env-variable-for-openml-cache branch October 14, 2024 16:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Environment variable for openml cache

5 participants