Skip to content
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

Document "exported" preferences #48

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

timholy
Copy link
Member

@timholy timholy commented Dec 2, 2022

This gives examples showing how to use "exported"
(I prefer "package-wide") preferences. It also reorganizes
the README around the major decision-points of
project- vs package-specific, and runtime vs compiletime.

This gives examples showing how to use "exported"
(I prefer "package-wide") preferences. It also reorganizes
the README around the major decision-points of
project- vs package-specific, and runtime vs compiletime.
@codecov
Copy link

codecov bot commented Dec 2, 2022

Codecov Report

Merging #48 (508e951) into master (09402bc) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master      #48   +/-   ##
=======================================
  Coverage   85.93%   85.93%           
=======================================
  Files           2        2           
  Lines         128      128           
=======================================
  Hits          110      110           
  Misses         18       18           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Comment on lines +38 to +40
If you call `load_preference` (or its macro variant `@load_preference`) from "top-level" in the package,
this is a compile-time preference. Otherwise (e.g., if it is "buried" inside a function, and that
function doesn't get executed at top-level), it is a run-time preference. See examples in the first API section below.
Copy link
Member

Choose a reason for hiding this comment

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

This isn't exactly true; a preference can still be loaded from within a function and used at compile time if that function is invoked during precompilation time.

Copy link
Member Author

Choose a reason for hiding this comment

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

"and that function doesn't get executed at top-level"? But obviously it's not clear.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, yeah. I'm honestly not sure it matters much to draw a distinction between compile-time and non-compile-time preferences; perhaps we should just have a note that says "if you use a preference at compile time, you will have to restart Julia before the changes will be seen" and leave it at that?

Copy link
Member

Choose a reason for hiding this comment

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

Alternatively, perhaps we could instead have set_preferences() spit out a @warn() when it notices a compile-time preference has been changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

I like that idea!

We still might need to discuss this issue somewhere, otherwise people might be mystified about why it sometimes requires a restart and why it sometimes doesn't. (Might feel like a bug when it doesn't warn.)

@@ -80,6 +94,41 @@ end
end # module UsesPreferences
```

With the macros, all preferences are project-specific.
Copy link
Member

Choose a reason for hiding this comment

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

Do you think it would be better to figure out how to pass kwargs to a macro, so we can have @set_preferences!("foo" => "bar"; export=true), or something like that?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that would make sense. A related issue: currently you have to read the docstring for set_preferences! in order to understand the force comment in the docstring for @set_preferences!. What do you want to do about that?

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, I don't understand active_project_only well enough myself to even try describing it in the README (I could read the code, but...)

@timholy
Copy link
Member Author

timholy commented Dec 4, 2022

Hmm, actually "package-wide" preferences don't work. Create a dummy package, WithPreferences, with these contents:

Project.toml:

name = "WithPreferences"
uuid = "dcf8c38c-589d-4550-bcab-b56e5c9171f3"
authors = ["Tim Holy <tim.holy@gmail.com>"]
version = "0.1.0"

[deps]
Preferences = "21216c6a-2e73-6563-6e65-726566657250"

[preferences.WithPreferences]
awesome_languages = ["Julia"]

src/WithPreferences.jl:

module WithPreferences

using Preferences

const awesome_languages = String[]

function loadprefs()
    prefs = @load_preference("awesome_languages")
    if prefs !== nothing
        append!(awesome_languages, prefs)
    end
end

end

Works if you're using the package's Project.toml as your environment:

tim@diva:/tmp/pkgdemo/dev/WithPreferences$ JULIA_DEPOT_PATH=/tmp/pkgdemo juliabin-1.8 --project -q
julia> using WithPreferences

julia> WithPreferences.loadprefs()
1-element Vector{String}:
 "Julia"

julia> WithPreferences.awesome_languages
1-element Vector{String}:
 "Julia"

Does not if you use your default environment:

tim@diva:/tmp/pkgdemo/dev/WithPreferences$ JULIA_DEPOT_PATH=/tmp/pkgdemo juliabin-1.8 -q
julia> using WithPreferences

julia> WithPreferences.loadprefs()

julia> WithPreferences.awesome_languages
String[]

I'm assuming it's supposed to work in the second case, but if not then I'd love to know your thoughts on whether it would be useful & worthwhile to support "package-wide" preferences and if so how to design it.

@timholy
Copy link
Member Author

timholy commented Dec 4, 2022

Also, with the current design, won't package-specific preferences need to be reconfigured each time you update to a new version? I see why that might be necessary, but it also seems likely to be frustrating for packages with complex configuration. Or even just "why is it back to using the xyz backend, I thought I just configured that!"

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.

2 participants