search

cjdoris / PreferenceTools.jl

Julia preferences for humans
MIT License
36 stars 3 forks source link

add get_all_prefs convenience function #5

Open kpa28-git opened 2 months ago

kpa28-git commented 2 months ago

The current get_all does not convert keys to symbols and symbol values stored as strings to actual symbols. As a result, the dictionary returned is not splattable and often not usable in manually setting preferences.

get_all_prefs provides a simple way to set preferences when a package does not support Preferences.jl yet, for example:

julia> Plots.theme(PreferenceTools.get_all_prefs("PlotThemes")[:theme])
julia> Plots.default(;PreferenceTools.get_all_prefs("Plots")...)
cjdoris commented 2 months ago

I'm OK with adding a function that returns a Dict{Symbol,Any} instead but the name get_all_prefs is too general - it's not clear how it's different from get_all. Perhaps you could just add a method get_all_prefs(::Type{Dict{Symbol,Any}}, pkg::AbstractString; kw...) so you can do get_all(Dict{Symbol,Any}, "Plots").

Also the conversion from ":foo" to :foo that you do is quite non-standard, but I see why you want it. Can you put it behind a keyword argument (default false) on get_all please? Perhaps allow_symbols or something?

kpa28-git commented 1 month ago

Yeah I originally called it get_all_splat, but I realized that was also unclear.

get_all_prefs(::Type{Dict{Symbol,Any}}, pkg::AbstractString; kw...) so you can do get_all(Dict{Symbol,Any}, "Plots").

To be clear you mean get_all(::Type{Dict{Symbol,Any}}, pkg::AbstractString; kw...)? Sure. I avoided overloading, but if you say it's no issue it works for me.

Also the conversion from ":foo" to :foo that you do is quite non-standard, but I see why you want it.

Do you mean the way I do the conversion or the conversion itself?

I can add a keyword arg. I'd rather have it default true because if someone calls get_all(::Type{Dict{Symbol,Any}}, ...) I believe it makes more sense to assume they would want all symbols converted to their native type (ie to actually be used as function arguments, etc).

But I guess I also see the point that it would diverge from how normal get_all behaves.

kpa28-git commented 1 month ago

I made the changes and added a readme example, please let me know what you think