add ability to shift moving average on plots

[deltreey]

For techniques such as the alligator, it is useful to be able to shift your moving averages. This PR creates the ability to make the mav parameter into a dict with 2 keys,

1. scale is the original mav values (tuple, int, or list)
2. shift is another set of values (tuple, int, or list) that must match the length of scale and shifts the moving average according to the value

Note: this PR does not change the way the mav parameter worked already. It only enhances it by adding this additional option

I created a test as well.

[deltreey]

don't know why the tests are failing. The image comparison works locally. Is there a setting I can change to be more like the CI environment?

[DanielGoldfarb]

Ted,

don't know why the tests are failing. The image comparison works locally. Is there a setting I can change to be more like the CI environment?

I believe the image test is failing because it is a new test and the reference image is not yet in the production repository. As long as the new test is the only one failing, and you say it is succeeding locally then all is good.

The other check that is failing is:
The command "/bin/bash scripts/check_version.sh $TRAVIS_PULL_REQUEST" exited with 1.
This is just a check to ensure that each PR bumps the version number. Please increment https://github.com/matplotlib/mplfinance/blob/master/src/mplfinance/_version.py from 17 to 18.

---

I would like to understand the choice of the dict keyword scale. I skimmed through the alligator reference and don't see anything there about scale. Can you explain, or point to some documentation. Is this a standard term used for this type of moving average analysis? (Instinctively I might have chosen a term like periods or num_periods).

--Daniel

[deltreey]

Ted,

don't know why the tests are failing. The image comparison works locally. Is there a setting I can change to be more like the CI environment?

I believe the image test is failing because it is a new test and the reference image is not yet in the production repository. As long as the new test is the only one failing, and you say it is succeeding locally then all is good.

I don't know how I feel about that. I didn't have the resolve image in place and had a failure and it was a different error message. I put the resolve image in to get it to pass locally, and it is included in the PR.

The other check that is failing is:
The command "/bin/bash scripts/check_version.sh $TRAVIS_PULL_REQUEST" exited with 1.
This is just a check to ensure that each PR bumps the version number. Please increment https://github.com/matplotlib/mplfinance/blob/master/src/mplfinance/_version.py from 17 to 18.

Done.

I would like to understand the choice of the dict keyword scale. I skimmed through the alligator reference and don't see anything there about scale. Can you explain, or point to some documentation. Is this a standard term used for this type of moving average analysis? (Instinctively I might have chosen a term like periods or num_periods).

--Daniel

Changed to period. I didn't have a reason for the name scale other than that I couldn't think of a better one. I don't like num_periods because we're asking for the periods themselves, and I think period is better than periods just as it was mav not mavs as a single int is an option to pass here.

[DanielGoldfarb]

Ted,
I have some concerns about the _mav_validator() code. nothing a show-stopper; I could accept the PR as is, but I think I will jot my thoughts down here and ask for your feedback:

1. There is some repetitive code in that many of the same checks are done for period, and for shift, and for the case of isinstance(mav_value,(tuple,list,int))
2. There is an inconsistency in that period and shift do not limit number of moving averages, but the plain mav version does. I am inclined to either limit, or not limit, all cases. Admittedly a maximum of 7 moving averages was an arbitrary decision on my part, but to date no one has requested more. On the other hand, removing a limit may simplify the code: maybe let the user shoot themselves in the foot trying to plot dozens or even hundreds of moving averages.
3. It makes sense that period must be > 1, but it may be reasonable to have a shift == 1.

With the above in mind, I took a shot at rewriting _mav_validator(mav_value). Let me know what you think:

def _mav_validator(mav_value):
    '''
    Value for mav (moving average) keyword may be:
    scalar int greater than 1, or tuple of ints, or list of ints (each greater than 1)
    or a dict of `period` and `shift` each of which may be:
    scalar int, or tuple of ints, or list of ints.  `period` must be greater than 1
    while `shift` must be greater than 0.
    '''
    def _valid_mav(value,minimum=2):
        if not isinstance(value,(tuple,list,int)):
            return False
        if isinstance(value,int):
            return isinstance(value,int) and value >= minimum
        # Must be a tuple or list here:
        for num in value:
            if not isinstance(num,int) and num >= minimum:
                return False
        return True

    if not isinstance(mav_value,(tuple,list,int,dict)):
        return False

    if not isinstance(mav_value,dict):
        return _valid_mav(mav_value)

    else: #isinstance(mav_value,dict)
        if 'period' not in mav_value: return False

        period = mav_value['period']
        if not _valid_mav(period): return False

        if 'shift' not in mav_value: return True

        shift  = mav_value['shift']
        if not _valid_mav(shift,1):  return False
        if isinstance(period,int) and isinstance(shift,int): return True
        if isinstance(period,(tuple,list)) and isinstance(shift,(tuple,list)):
            if len(period) != len(shift): return False
            return True
        return False

[deltreey]

Ted,
I have some concerns about the _mav_validator() code. nothing a show-stopper; I could accept the PR as is, but I think I will jot my thoughts down here and ask for your feedback:

1. There is some repetitive code in that many of the same checks are done for `period`, and for `shift`, and for the case of `isinstance(mav_value,(tuple,list,int))`

2. There is an inconsistency in that `period` and `shift` do not limit number of moving averages, but the plain `mav` version does.  I am inclined to either limit, or not limit, all cases.  Admittedly a maximum of 7 moving averages was an arbitrary decision on my part, but to date no one has requested more.  On the other hand, removing a limit may simplify the code: maybe let the user shoot themselves in the foot trying to plot dozens or even hundreds of moving averages.

3. It makes sense that `period` must be > 1, but it may be reasonable to have a `shift` == 1.

With the above in mind, I took a shot at rewriting _mav_validator(mav_value). Let me know what you think:

def _mav_validator(mav_value):
    '''
    Value for mav (moving average) keyword may be:
    scalar int greater than 1, or tuple of ints, or list of ints (each greater than 1)
    or a dict of `period` and `shift` each of which may be:
    scalar int, or tuple of ints, or list of ints.  `period` must be greater than 1
    while `shift` must be greater than 0.
    '''
    def _valid_mav(value,minimum=2):
        if not isinstance(value,(tuple,list,int)):
            return False
        if isinstance(value,int):
            return isinstance(value,int) and value >= minimum
        # Must be a tuple or list here:
        for num in value:
            if not isinstance(num,int) and num >= minimum:
                return False
        return True

    if not isinstance(mav_value,(tuple,list,int,dict)):
        return False

    if not isinstance(mav_value,dict):
        return _valid_mav(mav_value)

    else: #isinstance(mav_value,dict)
        if 'period' not in mav_value: return False

        period = mav_value['period']
        if not _valid_mav(period): return False

        if 'shift' not in mav_value: return True

        shift  = mav_value['shift']
        if not _valid_mav(shift,1):  return False
        if isinstance(period,int) and isinstance(shift,int): return True
        if isinstance(period,(tuple,list)) and isinstance(shift,(tuple,list)):
            if len(period) != len(shift): return False
            return True
        return False

I like it, but you may want to do a 0 or negative shift as well. I changed your code slightly to only do the > 1 check if the value is a period.

[DanielGoldfarb]

@deltreey
Ted,
Looks good. Thank you very much for contributing this code to mplfinance!

I'm going to merge this PR shortly. Please consider making a small .ipynb file for the examples directory, to show a few use cases of doing mav both with and without shift, and to explain advantages of shifting moving averages, and how it can assist with trade decisions.

All the best. Thanks. --Daniel