-
Notifications
You must be signed in to change notification settings - Fork 22
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
We are not clear on what happens when any given boolean parameter is omitted #115
Comments
I think there's a few ways to approach this, but I wasn't totally sure what you had in mind. If we don't make elements in the config mandatory, then the simplest would be to announce whenever we use a default value in the XSLT for the configuration file:
That won't make it clear to people when they're creating their config what the default option is, but it would at least alert them to what happened after the fact. However (and I'm not sure if you were suggesting this), we could also add the values as defaults to the schema and use those to populate the values in create_config.xsl. If we do add defaults, I think the XSLT should still output an INFO message when it uses the default value from the ODD. In either case, we should add a bit of a summary of the configuration options to the documentation. I like the way we have the information now, but I think a summary at the top would be helpful--I'm thinking a table with the columns However, I like the idea of making the configuration elements mandatory, but in my mind, it would be really complex since a number of options depend on other options: for instance, if you've set createContexts to false, then you shouldn't need to include any of maxKwicsToShow, maxKwicsToHarvest, phrasalSearch, wildcardSearch, totalKwicLength, or kwicTruncateString. That said, I'm not confident that we aren't doing that already. So if we were to make configuration options mandatory, I'd say we should do it properly: make sure we have all of the proper constraints in the ODD and ensure that our configuration XSLT implements that properly. That's a major project, of course, but one that will probably be a good thing in the long run. |
As a general rule, I prefer mandatory elements that you have to pay attention to over mysterious defaults that make things happen in the background without your knowing about them. We could use Schematron to enforce the co-occurrence constraints, and catch inconsistencies before the build goes ahead. I've been thinking about how complicated our configuration file is getting these days, and wondering about this:
|
Agreed.
That seems like a good idea to me--of course, adding schematron validation will add a bit of complexity to the build (generating the XSLT etc etc), but that seems worth it. We should also add the standard processing-instructions to the top of the config file so that people get all the benefits of the schema if/when they write them by hand. There will be some backwards compatibility issues. For instance, our test config would be invalid since it includes the
That all sounds good to me. When I first saw this ticket, I was thinking a similar thing re: a config helper (though I was thinking of a command line utility rather than via our documentation). But an HTML version will be much more helpful since all of the documentation will be right there; the only advantage of a command line thing is it could catch whether or not your search directory actually exists, but we could conceivably do that with schematron. |
@joeytakeda I'm not sure that we have a clear idea how to proceed on this, so I'm inclined to kick it to 1.3, where we'll probably also address the issue of a config helper tool. What do you think? |
Makes sense to me! I’ll move the milestone now |
I've created branch issue-115-experiment to play around with some options here. My first approach is going to be making all the elements mandatory, and giving them all default values in the schema. |
@joeytakeda I haven't done a pull request for this yet, but I think in general the approach taken in the issue-115-experiment branch is the only one which will really work; specifying defaults in the ODD is tricky for many of these values, and it might just be easier to give people a template to start from where they're all included with their defaults. What do you think? |
@martindholmes I'm mostly in agreement here with two exceptions: Making As I think I've said before (though I can't find it now), I'm not a fan of requiring configuration options that So one option is that |
@joeytakeda The I take your point about On the issue of options made irrelevant by other options, I take your point, but I don't see that it does any harm. I think if we were starting from scratch, these options would be organized as attributes on a smaller number of elements, giving us better configuration possibilities in terms of default values, but I don't think it's worth changing now. But I wonder if it's possible to re-imagine the config options in such a way that they always make sense. We could do that by figuring out the minimum number of bits of information that the build needs to know, and see if there are more elegant ways to group the data. Anyway, this branch is not useful at this point, and probably couldn't be merged without a lot of work after the weekend scramble to get 1.3 out. I should probably delete it or rebase it on the current dev. |
I think empty
LOI would fall into that camp, I think, but wildcards are essential because of variant spellings, of course. I presume the same would go for VI History, but I can see how that might be a case where the "global" search is very basic to keep a reasonably sized index and then the "advanced" sub-collection searches have wildcards enabled, etc.
I agree that we should really think about better ways to group things--whether that's elements with attributes or containers around particular types of elements or just co-occurence constraints in Schematron. |
We don't have default values in the content models of our booleans parameters, and we are not consistent with our defaults:
(from create_config.xsl).
I think our defaults are common-sensical, generally, but it should be clear what you're getting if you don't include one of these optional elements. Another alternative would be to make all the elements mandatory.
The text was updated successfully, but these errors were encountered: