Wow, thank you all for the responses! This is so helpful!
First off, I’ll definitely put a prominent note in the docs (readme probably) that the API is not stable yet.
@willingc Yes, this is the first project that I am maintaining, I’m learning a lot as I go and so far pyopensci has been very helpful!
@NickleDave Yes, I had considered having the user pass in FormatOptions, this is similar to @CAM-Gerlach’s suggstion so I’ll address it there.
@CAM-Gerlach wow, thank you for this response. You 100% captured what I’m trying to accomplish, what I’ve tried, and you’ve spelled out exactly the design constraints that I’ve been weighing in my head, but in a much more clear way than I was thinking about them.
I had previously toyed around with having the user construct and pass in FormatOptions to the Formatter and global settings configuration functions but there were two issues.
- It puts a little more work on the part of the user. They have to learn about the
FormatOptionsclass in addition to the other functions and, in code, they have to perform the extra step of creating aFormatOptionsclass before they can create aFormatteror configure global options. - I struggled a little bit getting the logic correct to have the default arguments injected from the global defaults. I think I had some ideas but they felt clunky.
Point 2 is thoroughly addressed by CAM-Gerlach’s suggestion about using a render method that converts a partially initialized set of options into a fully initialized set of options, patching with the global defaults. In fact the suggestion there is an improvement over what I have now because right now, for the Formatter, the global defaults are applied at initialization time rather than format time*. Of course either behavior could be correct depending on how I want it to be, but I think applying the defaults at format time makes more sense.
Regarding point 1. As you point out, there is a trade off between exposing the individual arguments for user convenience and reducing duplication/complexity in the codebase. But I think I’m agreeing with you and @NickleDave that this trade off is not really that bad for users. For the way I use the code and imagine a lot of others using it it results in an extra line of code to setup the FormatOptions before passing it into a single Formatter used for the whole script. But, I think it can also give the users nice flexibility in some ways. For example, right now the FormatOptions class is immutable so the Formatter is immutable. But maybe I could make the partially initialized UserFormatOptions mutable so that users could mutate it (or if I want it to also be immutable then users could have an easy way to duplicate + modify) and pass it around on the fly if they have reason to use multiple different sets of options (this needs to be thought about a little carefully). I could also make some sets of anticipated or user-requested “standard” option sets beyond just the package defaults that users could import and use in their formatters. I could even include Formatters pre-configured with these standard options so that users lucky enough to want to use one of these standard options sets can skip the “construct FormatOptions” step (of course something like this could probably be done the way the code is currently structured, but thinking about FormatOptions in this way made the idea feel a little more natural).
For the record, it feels that a little bit of convenience is lost on the user-facing end for some of the simple use cases, but, still on the user-facing end, a little bit might be gained by thinking in terms of passing around FormatOptions objects and, as discussed, a lot will be gained on the back-end in terms of complexity and but potential.
I will definitely try out what’s been suggested here and I think there’s a good chance I’ll go with it. I’ll probably go with the recommended option involving two classes, one partially filled and the other totally filled.
@CAM-Gerlach one final general question coming from someone still moving out of the shallow end of the code development pool about a comment you made. You said “you might be able to find a much more optimal point in terms of reducing duplication while not regressing or even improving elsewhere.” What exactly do you mean by “regressing”? The suggestion to require users to pass in some sort of FormatOptions class instead of direct keyword arguments will allow all of the core functionality to remain in tact, but, technically, my tests will fail under this change since they use the old API. the tests will need a trivial update to use the new API. Is this considered a regression or not? Maybe not since it is an intentional API change, announced in the release, so the test API is expected to need to change?
*When using the SciNum class and formatting via the format specification mini language the global defaults are applied at format time.