Options class replacement

[johnomotani]

As discussed here #25 (comment) there are various issues with using the options module for handling settings in hypnotoad:

@dschwoerer

• I just wanted to mention that options pulls in additional dependencies. As options doesn't even seem to have a public bug tracker (same is true for some dependencies) I do worry about pulling that in ...
• options allows getting options simply as e.g. self.options.orthogonal, but "that is only a few lines [to implement], I [@dschwoerer] did that to drop the dependency on Bunch."

@ZedThree

• not being able to deepcopy
• removing keys from the passed-in object
• To make the options form in the gui more useful, I think we would want to add some additional features to the options class at any rate:
  • keeping track of documentation
  • expected type of values, allowed values. For example, make refine_method into a drop-down list of the available methods.

A replacement Options class would be welcome. Maybe a possibility for something shared with xBOUT boutproject/xBOUT#94 boutproject/xBOUT#97?

[bendudson]

I agree with the points above.

In general I think the idea of a persistent / immutable data structure for options is a good one, but options is probably too flexible: It provides too many different ways to update or modify values (mutating or non-mutating? set, push, add, addflat), and the distinction between them is not well documented. When seeing it used, it was not always clear to me where values were set, given the multiple levels where defaults can be provided or overridden. It's possible that python is too flexible to enforce a particular mental model of what is going on, but I think it would be nice to have a simpler system. Personally I'd like it to be immutable (no set operations, all changes are effectively deepcopies), or at least allow deep copies.

[bendudson]

For modifying nested data structures (e.g. maps containing maps), the JS immer library has a quite nice interface: https://immerjs.github.io/immer/docs/introduction. I don't know how hard that might be to implement in python though, especially with the limitations of python lambdas. Things are simpler if options is a flat dict/map structure, so options can't contain options.

[johnomotani]

Personally I'd like it to be immutable (no set operations, all changes are effectively deepcopies), or at least allow deep copies.

It might actually be nice to have the options be immutable by default, but be able to say that some are allowed to be changed. I think mostly the options have to be set when the Mesh object is created (nx and ny for regions for example) but for nonorthogonal grids there's a subset of options (the nonorthogonal_* ones) that can be usefully changed before calling MeshRegion.distributePointsNonorthogonal again: the Mesh.redistributePoints method and regrid button in #26 demonstrate how this is a useful workflow.

Edit: it might be better to have two different options objects in Equilibrium - one immutable and one mutable, then it would be obvious which option was which and straightforward to have two different options tables in the gui - one changes immutable options, so you have to click Run again afterwards, and one changes mutable options, so you can just click Regrid.

[johnomotani]

(not really the ideal place for this, but want to keep it in an open issue somewhere)

#24 (comment)

Future wishlist: raise OptionError with name of option as an attribute so we can highlight bad options?

[bendudson]

Possibly Probably Definitely not ideal, but here's a quick idea of what a replacement could look like:

class Options:
    __isfrozen = False  # Allow changes during initialisation
    
    def __init__(self,**kwargs):
        self.__data = kwargs
        self.__isfrozen = True  # Disable further updates
    
    def keys(self):
        return self.__data.keys()
    
    def __getitem__(self, key):
        return self.__data.__getitem__(key)

    def __getattr__(self, key):
        return self.__getitem__(key)

    def __setattr__(self, key, value):
        if self.__isfrozen:
            raise TypeError("'Options' object does not support item assignment")
        object.__setattr__(self, key, value)
    
    def using(self, **kwargs):
        """Return a copy, overriding given values"""
        newdata = self.__data.copy()
        for key in kwargs:
            newdata[key] = kwargs[key]
        return Options(**newdata)

    def without(self, *omit_keys):
        """Return a copy without the given keys"""
        newdata = {}
        for key, value in self.__data.items():
            if key not in omit_keys:
                newdata[key] = value
        return Options(**newdata)
    
    def __str__(self):
        return self.__data.__str__()
    
    def __repr__(self):
        return "Options(" + ", ".join([str(key) + "=" + repr(value)
                                       for key, value in self.__data.items()]) + ")"

obviously with comments, tests. This allows it to be treated as a dictionary (or Bunch, with the getattr method), except that changes don't propagate upstream. For example:

a = Options(key=2, test=42)

a["test"] # -> 42
a.test # -> 42

# Create a new Options, adding new keys
b = a.using(test = 3)

b # -> Options(key=2, test=3)

# Create a new Options, omitting some keys
c = a.without("test")
c # -> Options(key=2)

# a is unchanged. Attempts to modify it directly lead to errors
a.key = 4 # TypeError
a["test"] = thing  # TypeError
del a["key"]  # TypeError

Where there are default values that the user is overriding, this would be used as:

defaults = Options(...)

user_settings = ... # Dict or Options, or other mappable

settings = defaults.using(**user_settings) # Override defaults