fix: More intuitive min_bound_range behavior

[FAB1150]

This is a quick and simple change to the behavior of the min_bound_range option, to make it more intuitive to use and more predictable to new users.

Old behavior: it ignores every other bound option and overrides it

New behavior: If lower bound is set, it increases the range "upwards". If upper bound is set, it increases the range "downwards". If no bound is set, or if both upper AND lower bounds are set, old behavior is used.

This was mentioned in #1047, not an essential groundbreaking feature but it was a simple enough fix that I gave it a try. Tested on my machine.

This is my second ever PR so go easy on me please :)

Did I use the correct semantic title?

[akloeckner]

Thanks a lot! I had a look at the code, and I think it looks good. 👍

Did you think about how soft bounds should best be handled? They're kind of in between undefined and fixed. Maybe they should be handled as if they were undefined?

I'll wait for @ildar170975's opinion, too, before merging.

Did I use the correct semantic title?

I would actually say, you're proposing a fix. You could edit the title to change this. A refactor would be, if you were changing the way different parts of the code interact with each other.

[akloeckner]

Thinking about the soft bounds... The if-then-else will become huge. Maybe something like this might work? (I haven't tested it at all!)

const weights = [
  min !== undefined && min[0] != '~' ? 0 : 1,
  max !== undefined && max[0] != '~' ? 0 : 1,
];
const sum = weights[0] + weights[1];
if (sum > 0) {
  boundary = [
    boundary[0] + diff * weights[0] / sum,
    boundary[1] + diff * weights[1] / sum,
  ];
} else {
  boundary = [
    boundary[0] + diff / 2,
    boundary[1] + diff / 2,
  ];
}

[ildar170975]

@akloeckner I will be available in a week (I hope).

[FAB1150]

@akloeckner I had thought of soft bounds, and didn't think it would have made a difference as this code gets executed later. I didn't consider the fact that in the scenario that a soft bound is exceeded, my code still treats it as a bound and has a weird behavior.

I'll test your code and edit this comment when I do! I see it always treats soft bounds as "not a bound", is this the behavior we want it to have?

[akloeckner]

is this the behavior we want it to have?

To be agreed. I'd say, soft bounds are bound (😄) to be broken. So, they can also be extended by the minimum range. Whereas fixed bounds should stay fixed as long as possible.

But you're right. Maybe one soft bound plus one free bound: the soft bound should stay and the free one should be extended? So, my code is not optimal either.

Sorry, if I'm over complicating...

[FAB1150]

@akloeckner

one soft bound plus one free bound: the soft bound should stay and the free one should be extended

That's what I was thinking, but maybe that's feature creep - your code looks good honestly (it just needed a !== instead of the != to avoid lint errors, and switching two + for -). I haven't really tested it yet as I was away, but it's compiled ready on my pc :)

EDIT: tested, and it works well! If I have time today I'll try to adapt the soft bound behavior, but no promises :D

tested working code:

      if (diff > 0) {
        const weights = [
          min !== undefined && min[0] !== '~' ? 0 : 1,
          max !== undefined && max[0] !== '~' ? 0 : 1,
        ];
        const sum = weights[0] + weights[1];
        if (sum > 0) {
          boundary = [
            boundary[0] - diff * weights[0] / sum,
            boundary[1] + diff * weights[1] / sum,
          ];
        } else {
          boundary = [
            boundary[0] - diff / 2,
            boundary[1] + diff / 2,
          ];
        }
      }

[FAB1150]

I thought about it for a bit, and came up with this:

const weights = [
  min !== undefined && min[0] !== '~' ? 0 : (max === undefined && min !== undefined ? 0 : 1),
  max !== undefined && max[0] !== '~' ? 0 : (min === undefined && min !== undefined ? 0 : 1),
];

Now if it's defined but a soft bound, and there is no other bound defined, it treats it as a bound. If both are soft bounds then it doesn't care, and if the other bound is a hard bound, the soft one is breached. Tested it on my machine and it works great :)

I'll add this to the PR, let's see if we want to treat the soft bounds like this.

EDIT: now that I think of it, we could also remove the second"is it defined" check:

const weights = [
  min !== undefined && min[0] !== '~' ? 0 : (max === undefined ? 0 : 1),
  max !== undefined && max[0] !== '~' ? 0 : (min === undefined ? 0 : 1),
];

as if both boundaries are undefined, they both have a weight of 0 and it uses the old behavior as intended.

[FAB1150]

@akloeckner tested that too, great! now it's more elegant.