-
-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[FIX] main: Fix widget settings directory path to clear #3932
[FIX] main: Fix widget settings directory path to clear #3932
Conversation
Since biolabgh-3772 the `environ.widget_settings_dir` is no longer the authoritative directory path. Change the application name to 'Orange' to preserve historical directory layout (the application name is used as the path component in the settings path).
Codecov Report
@@ Coverage Diff @@
## master #3932 +/- ##
==========================================
+ Coverage 84.71% 84.98% +0.26%
==========================================
Files 371 376 +5
Lines 64929 66461 +1532
==========================================
+ Hits 55006 56480 +1474
- Misses 9923 9981 +58 |
I am still a bit confused. Which is the correct one to use (and why aren't all others deprecated)? >>> Orange.version.version
'3.23.0.dev0+15d4fe6'
>>> Orange.canvas.config.widget_settings_dir()
'/home/lan/.local/share/Orange/3.23.0.dev/widgets'
>>> Orange.widgets.settings.widget_settings_dir() # Wrong version?
'/home/lan/.local/share/Orange/0.0.0/widgets'
>>> Orange.misc.environ.widget_settings_dir() # Now deprecated
'/home/lan/.local/share/Orange/3.23.0.dev/widgets'
>>> Orange.canvas.utils.environ.widget_settings_dir # str not function! Module deprecated
'/home/lan/.local/share/Orange/3.23.0.dev/widgets' |
Orange/misc/environ.py
Outdated
return os.path.join(data_dir(versioned=versioned), "widgets") | ||
warnings.warn( | ||
f"'{__name__}.widget_settings_dir' is deprecated. " | ||
"Use 'Orange.widgets.settings.widget_settings_dir'", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be 'Orange.canvas.config.widget_settings_dir'
?
And 'Orange.widgets.settings.widget_settings_dir'
removed since it is wrong?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Orange.widgets.settings.widget_settings_dir
is the effective directory (technically it is orangewidget.settings.widget_settings_dir
. It will become the correct path once the application starts and configures.
>>> Orange.widgets.settings.widget_settings_dir()
'/Users/aleserjavec/Library/Application Support/Orange/0.0.0/widgets'
>>> Orange.canvas.config.Config().init()
'/Users/aleserjavec/Library/Application Support/Orange/3.23.0.dev/widgets'
This actually has to happen very soon before the widgets are imported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, so after configuration they both return the same correct path. But still, when should one use which
Orange.canvas.config.widget_settings_dir
Orange.widgets.settings.widget_settings_dir
and could we have just one?
Currently they also have different calls to orangewidget:
orangewidget.workflow.config.widget_settings_dir
orangewidget.settings.widget_settings_dir
I think the public API of orangewidget should have one path from which to import widget_settings_dir.
If the same thing is available from multiple locations (either in orangewidget or in orange) it just creates confusion and provides opportunities for problems in the future IMO.
Issue
Fixes gh-3931
Since gh-3772 the
environ.widget_settings_dir
is no longer the authoritative directory path.Description of changes
Orange.misc.environ.widget_settings_dir
, (but make it return the correct path).Includes