Modifications in response to issue #52 to go back to original call si…

[jranalli]

closes #52. Modifications were made as simply as possible and used a similar method to argparse.

Check for the presence of the variable of interest in vargs, and if it's there, move it into kwargs. Then call the followup function without referencing vargs. Only applies when p is the first positional argument, because argparse already assumes T is the first positional argument.

        if len(varg) > 0:
            if 'p' in kwarg:
                raise pm.utility.PMParamError('p was specified both positionally and with a keyword.')
            kwarg['p'] = varg[0]
        if len(varg) > 1:
            raise pm.utility.PMParamError('There are only two positional arguments: h, p.')

Some test code to ensure reliability. Any other calls already rely on kwargs and should be fine.

def test_bug_git52(self):
        # ig, ig2, igmix, mp1
        subs = [pm.get("ig.BH3O3"), pm.get("ig.O2"), pm.get('ig.air'), pm.get('mp.H2O')]

        for sub in subs:
            p = 1
            T = 500
            h = sub.h(T=T, p=p)
            s = sub.s(T=T, p=p)
            assert sub.T_s(s, p) == approx(sub.T_s(s, p=p))
            assert sub.T_s(s, p) == approx(sub.T_s(s=s, p=p))
            assert sub.T_h(h, p) == approx(sub.T_h(h, p=p))
            assert sub.T_h(h, p) == approx(sub.T_h(h=h, p=p))
            if hasattr(sub, "p_s"):
                assert sub.p_s(s, T) == approx(sub.p_s(s, T=T))
                assert sub.p_s(s, T) == approx(sub.p_s(s=s, T=T))
            if hasattr(sub, "d_s"):
                assert sub.d_s(s, T) == approx(sub.d_s(s, T=T))
                assert sub.d_s(s, T) == approx(sub.d_s(s=s, T=T))

[chmarti1]

I have a simpler suggestion. Let's just do this:

def T_s(self, s, p=None):
    if p is not None:
        return self.T(s=s,p=p)
    return self.T(s=s)

That lets the interpreter handle parsing the arguments and throwing errors automatically.

[jranalli]

As I'm looking at it I think we want to retain the kwarg part. If the goal is to match the old signatures for T_s, etc. I think you need to retain the quality=True flag for mp1, and the ability to use density.

So I think for mp1 it would be like:

        if p is not None:
            T,_,_,x,_ = self._argparse(s=s, p=p)
        else:
            T,_,_,x,_ = self._argparse(s=s, **kwarg)
        if quality:
            return T,x
        return T

And for ig:

        if p is not None:
            return self.T(s=s, p=p)
        return self.T(s=s, **kwarg)

Does that look right to you? If so, I'm happy to make the edits.

[chmarti1]

True. I think I'd like to weave this into my own working branch and play with it a bit before we fold it in.

[jranalli]

Ok, just committed the changes that would follow the methodology proposed there if you want to use these as a simpler starting point. Or you can just back off by a commit or start from wherever you like!

[chmarti1]

Whew. It really took me a while to get back to this. I agree with your edits - this is the right way to do it. I'm approving this pull request.