Convert dir to axis

[cako]

Motivation

As described in #123, the choice of dir for direction in many linear operators is not compatible with NumPy/Scipy notation. In addition, it encroaches on the dir built-in function. As such, it is highly desirable to move from the use of dir to axis instead. In addition, some other related keywords such as dirs and nodir can both be substituted by axes. While there are other improvements that can be made in this respect, the scope of this PR is simply to move away from dir without altering the current functionality of these modified operators.

Strategy

The main issue to keep in mind is to retain the old behavior as much as possible, so as to allow users to migrate to axis and not break any existing code. The approach taken to ensure this was as follows

1. Change function signatures from ..., dir=0,... → ..., axis=-1, dir=None, ... and similar for dirs/axes.
2. Ensure that when dir is supplied, it supersedes the use of axis but also issues a deprecation warning:

if dir is not None:
    warnings.warn(
        "dir is deprecated in version 2.0.0, use axis instead.", category=DeprecationWarning, stacklevel=2,
    )
    axis = dir
else:
    axis = axis

3. The documentation is updated to always use axis and note that dir is deprecated like so:

axis : :obj:`int`, optional
    .. versionadded:: 2.0.0
    Axis along which derivative is applied.
dir : :obj:`int`, optional
    .. deprecated:: 2.0.0
        Use ``axis`` instead. Note that the default for ``axis`` is -1
        instead of 0 which was the default for ``dir``.

4. Changes to the linear operators were first made prior to changing tests: this ensured during development that using dir always worked. After this check, tests were updated to also call axis.

Considerations

I am creating this as a draft because there I'd like some feedback on this strategy. For example, I strongly favor setting the default as axis=-1 for v2.0.0, but this is a breaking change. I am also open to amending warnings/documentation on this change.

[mrava87]

Let me get back to this once we got v1.17.0 out :)

[mrava87]

I have not forgotten about this, just being thinking a little about it.

Overall thought:

• My main fear is that we are polluting the input variables with an another parameter and this may create more confusion than benefit in the short run. Postponing this to v2.0.0 and describe this as one of the major code-breaking changes may be a better option?

Here a some more specific thoughts:

• In general I agree the choice of dir was a bad one I regretted early. But I decided that a worst choice would have been to switch half way through... probably doing what you have done early on would have been the best choice ;)

• Similarly I agree that in v2.0.0 we should move to default axis=-1. I can think of a few cases where having dir=0 as default makes some sense for how the operator is operating, but in most cases working on the last axis is the best idea.

• One thing I am not sure I understand Change function signatures from ..., dir=0,... → ..., axis=-1, dir=None, ... and similar for dirs/axes.. I understand that a code currently working that has an operator where dir=0 will keep working. But what about a currently written code where an operator is made but dir is not specified, would the behavior suddenly change to axis=-1? Unfortunately if this is the case I think we need to introduce axis more softly... one way could be to have axis=None and if this is provided it will take priority... but maybe I don't understand the strategy you designed?

• In warning messages I think it is better to say dir will be deprecated as we don't have v2.0.0 yet.

[cako]

Hi @mrava87 thanks for the comments. Let's go over your them:

• I absolutely agree that this should go in v2.0.0. The question is, are you expecting to release other versions before v2.0.0? If so, we could move this to a feature branch tagged v2.0.0.
• Yup, maintaining compatibility is important!
• Also agree here, there may be some operators where axis=0 makes more sense as a default. I know NumPy has a few of those. If you think that any of the ones I changed should be changed, flag them and let's discuss!
• The signature change is kind of tricky. When setting a value as a default e.g. dir=0 in the function signature, there is no way to distinguish between calling the function with myfunction() or with myfunction(dir=0). In order to distinguish between the two cases, you have two options:

  def myfunction(dir=None): # Option 1
  def myfunction2(**kwargs): # Option 2

  Calling myfunction() will create dir=None in the function body and calling myfunction(dir=0) will create dir=0 in the function body. This way we can distinguish in non-None values are passed to the function. When calling myfunction2(), dir will not exist in kwargs and calling myfunction2(dir=any_value) will create kwargs[dir] = any_value.
  So if we want to ensure that dir is respected whenever it is supplied, we need to choose one of two options. Since we don't use dir=None as a valid keyword, the first option is preferred. Using kwargs is generally the last resort since it makes code really hard to read, maintain and debug (for example, calling myfunction2(arg_that_doesnt_exist=0) does not raise an error. That's the reasoning behind the dir=None.
  To answer your question, yes, not specifying dir will use axis=-1. But that's what a default of axis=-1 is. One possibility I considered was to first introduce axis=0 (the same as dir) and only later switch the default to axis=-1. But in this case, the user will have to know exactly which version introduces the change of defaults. On the other hand, doing it all at once ensures the user that if axis is a valid keyword, then axis=-1 will always be the default. In terms of priority, dir will always precede axis until it is retired as a keyword.
• Good point, I will change it!

[cako]

This PR, together with #332 closes #123.

[mrava87]

Great! Let me take a look in details in coming days (I just want to be sure if there is any operator where we don't want to switch convention 0->1) then I can merge it :)

[mrava87]

First half, done up to Interp

[mrava87]

Second part

[cako]

@mrava87, could you have a final look? By the way, we should probably squash all this into one commit when we merge :)

[mrava87]

Let me take a final look and I let you know if I have anything more :) Then I am happy if you want to squash all in one commit as there is quite a lot here and it all fits within the same idea of removing dir :)

[mrava87]

Alright, apart from the small comment on the MIGRATION file, everything else looks good to me and ready to go :)

[cako]

Alright, apart from the small comment on the MIGRATION file, everything else looks good to me and ready to go :)

Have a look here. If it's ok, feel free to merge!

[mrava87]

Wait, maybe my comment was confusing. In the MIGRATION file you say:

the default `dir` and `dirs` was -1 and (0, 1),

shouldn't that be:

the default `dir` and `dirs` was 0 and (0, 1),

Apart from that, you want to squash all in one commit before I merge or not? I don't really have strong feeling here ;)