Stdpopsim: modifying existing stdpopsim objects

[mufernando]

Many of stdpopsim objects are mutable, but most of the times we have decorators to validate attributes only at or right after initialization (e.g. here). Users can modify existing objects by accessing their attributes, as in:

species = stdpopsim.get_species("HomSap")
contig = species.get_contig("chr22", length_multiplier=0.1)
contig.mutation_rate = 10

If one of our validation checks was that mutation rates have to be less than 1, it would not be triggered and the user would produce an object that is not valid anymore. There doesn't seem to exist a nice way of dealing with this; see this: https://github.com/python-attrs/attrs/issues/109.

We could, however, freeze all objects and only allow reuse via attr.evolve. This is a bit more cumbersome for users, but it is the easiest way to allow user modification while ensuring objects are valid.

Any thoughts or ideas?

cc @grahamgower

[grahamgower]

The suggestion to recommend using attr.evolve() is an excellent one! I'm on the fence about having frozen objects though... on the one hand: yes it would be good to always validate the attributes; but on the other hand frozen attributes can be confusing for those unfamiliar with attrs (probably most users), we don't seem to be doing a great deal of validation anyway (I didn't see anything outside ext/selection.py), and frozen attributes aren't truly immutable anyway. But maybe this just means we should be doing more attribute validation?

[jeromekelleher]

Any time we've frozen an attribute someone has come up with a good reason for wanting to mutate it. I'm not sure there's much value in trying to be too strict about this, we'll get errors from the simulation engines anyway if values people put in are silly, right?

[grahamgower]

we'll get errors from the simulation engines anyway if values people put in are silly, right?

Most likely. For some background here, @mufernando is planning on adding new attributes to the Contig class over in #644. Probably it will be tricky for users to alter these attributes themselves in any sensible way.

[jeromekelleher]

Probably it will be tricky for users to alter these attributes themselves in any sensible way.

I like the Python philosophy here: it's more important to help users write good programs rather than prevent them from writing bad programs. So, unless there's a strong safety argument (i.e., things will explode or the program will silently return misleading or bad results) I wouldn't put too much energy into policing the interface.

[mufernando]

Thanks for the input @jeromekelleher and @grahamgower!

At this point I'm not sure whether the program would return misleading or bad results, but I think it is possible that could happen. This could be particularly bad with the proportions of different mutation types. I could replicate some of the most crucial validation steps within simulate to mitigate that.

What do you think about in the docs only giving examples with attr.evolve, @jeromekelleher? I think this would mesh well with Python's philosophy, as we would be guiding them to writing code in a safer way.

[jeromekelleher]

We could document using attr.evolve I guess, but it does mean that end up having an explicit API dependency on attrs then, and we could never stop using it. That feels to me like a bigger compromise, but I'm not writing the code so :shrug:

[mufernando]

well, we don't need to worry about this any more. I provided a few methods that modify Contig and does the checking in #644 . This way users won't have to manually edit any of the attributes.