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

Should multiple option search_list_extensions concatenate? #52

Closed
tkeffer opened this issue Jul 13, 2015 · 9 comments
Closed

Should multiple option search_list_extensions concatenate? #52

tkeffer opened this issue Jul 13, 2015 · 9 comments
Labels

Comments

@tkeffer
Copy link
Contributor

tkeffer commented Jul 13, 2015

A user (reasonably) wanted to use the forecast search list extensions in the exfoliation skin. What was unusual is how he did it --- by overriding in weewx.conf:

[[Exfoliation]]
        skin = exfoliation
        HTML_ROOT = public_html/exfoliation
        [[[Extras]]]
            footer = /home/weewx/skins/exfoliation/footer.inc
            header = /home/weewx/skins/exfoliation/header.inc
            hilo = /home/weewx/skins/exfoliation/hilo.inc

        # MT Addition for forecast
        [[[CheetahGenerator]]]
            search_list_extensions = user.forecast.ForecastVariables

He also declared an additional SLE (the "alltime" example) in the exfoliation skin.conf, the conventional way

[CheetahGenerator]
    encoding = html_entities

    # MT added to provide alltime and seven_day tags
    search_list_extensions = examples.xsearch.MyXSearch

Unfortunately, the former masks the latter, so the tag $alltime did not work.

Here's the thread: https://groups.google.com/d/msg/weewx-user/MlD1mTUakIA/Ai8ua22g6oUJ

I must admit, I had not thought of a user overriding search_list_extensions from weewx.conf, but it seems like a reasonable thing to do.

A solution would be that the final SLE should be the concatenation of the two.
Alternatively,we could warn in the log that one is replacing the other.

@tkeffer tkeffer added the bug label Jul 13, 2015
@matthewwall
Copy link
Contributor

imho, concatenating would lead to even more confusion. every entry specified in weewx.conf overrides whatever is set in skin.conf. if we introduce concatenation, then we have to remember which parameters are overrides and which are concatenations.

(fwiw, i've been using this approach since i first wrote the exfoliation skin)

@tkeffer
Copy link
Contributor Author

tkeffer commented Jul 31, 2015

Then this suggests we should warn if a weewx.conf override is masking something in skin.conf.

BTW, I've been meaning to ask you, why do you override in weewx.conf? Why not just put the settings in the skin's configuration file? That makes it local.

@matthewwall
Copy link
Contributor

the exfoliation skin needs to work whether or not the forecast extension is installed, so the skin.conf assumes there is no forecasting. i'm pretty sure that the report will fail if it specifies a search list extension that is not installed.

so when you install the forecast extension, you need to add the search list extension to exfoliation. if you modify skin.conf then you have to propagate your changes each time to take an update to the exfoliation extension. if you specify them in weewx.conf, then exfoliation updates are trivial to apply.

@matthewwall
Copy link
Contributor

btw, the exfoliation skin no longer requires full paths to its include files. those were necessary before the chdir bug was fixed in the CheetahGenerator.

@tkeffer
Copy link
Contributor Author

tkeffer commented Jul 31, 2015

As I think about this, I think we can make the case that search list extensions are, well..., different from other parameters.

By its very nature, Cheetah uses the union of all the search lists you give it. So, if you specify an SLE in weewx.conf, then a different one in skin.conf, it would be reasonable to expect Cheetah to be able to use both of them.

@matthewwall
Copy link
Contributor

if parameters in weewx.conf are merged with parameters in skin.conf, then how does one override skin.conf with something in weewx.conf? what order will the resulting items be in?

@tkeffer
Copy link
Contributor Author

tkeffer commented Jul 31, 2015

The extensions specified in option search_list_extensions in weewx.conf would appear in the final search list before the ones specified in skin.conf. So, if a tag is defined in both places, the one in weewx.conf would win, just like all the other options.

@matthewwall
Copy link
Contributor

the problem with merge instead of override for search_list_extensions is that then there is no way to override.

as a practical matter, the combining of options from skin.conf and weewx.conf happens outside of each generator, in class StdReportEngine. so the cheetahgenerator would have to re-read the skin.conf then re-process parameters, with a special case for search_list_extensions. or the StdReportEngine merging would have to be refactored to enable the special case of search_list_extensions.

meanwhile, i added debug-level logging to cheetahgenerator that indicates exactly which objects are in the search list, as well as a list of the search list pseudo-dicts that are available for use in templates. that provides users with the ability to see the effects of their weewx.conf and skin.conf changes.

@tkeffer
Copy link
Contributor Author

tkeffer commented Oct 5, 2016

That seems like a reasonable first step. Let's close this issue --- we can revisit if necessary.

@tkeffer tkeffer closed this as completed Oct 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants