Closed ethanwhite closed 5 years ago
totally agree with your points and can see your issues here
i actually had the arguments in the other order (what you both expected), but had to change it for some coding reason i can't recall specifically right now (something to do with how the names are combined). this is definitely one situation where a slightly more complicated code under the hood of the function is worth it if it helps the api be that much easier to work with.
also, i think this model set configuration situation is a good type example of how the api can be improved by collapsing down some of the pipes now that all the parts of the codebase have been properly connected (and now that i've seen some cleaner examples of how to set up extensive options lists much more cleanly). i'm wondering if you two had other interactions with the code that you thought similarly about.
I think we've opened issues for everything that jumped out at us so far and will continue to do so. The one exception was the one I mentioned in briefly in #117, which was that for common tasks I'd prefer (if possible) to do tweaks as direct arguments in portalcasting
. In particular it seems like:
portalcast(models = c("mynewmodel", "ESSS"))
Would be a bit more user friendly, but if it's tricky under the surface it's workable the way it is.
addressed in #129
Both @skmorgane and I found the
model_names
API a bit confusing. We like the idea of having the ability to both have a prefab list and add additional models, so maybe it's just a docs things.UX issues that we experienced:
With that in mind our thought was that an API like this might be a good option:
So then users can do initial development like
Test against a primary competitor
And then do a full comparison to existing models