Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Curve.rebinned() errors when fixp is outside domain #109

Open
antnieszka opened this issue Dec 5, 2016 · 2 comments
Open

Curve.rebinned() errors when fixp is outside domain #109

antnieszka opened this issue Dec 5, 2016 · 2 comments

Comments

@antnieszka
Copy link
Contributor

antnieszka commented Dec 5, 2016

I think, we may have some inconsistency in Curve.change_domain() and Curve.rebinned().

class TestCurve(TestCase):
    def setUp(self):
        self.c = Curve([[0, 0], [5, 5], [10, 0]])

    # other tests...

    def test_rebinned(self):
        new_c = self.c.rebinned(step=1)
        assert np.array_equal(new_c.x, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10])
        
        # E1
        new_c = self.c.rebinned(step=2, fixp=15)
        assert np.array_equal(new_c.x, [1, 3, 5, 7, 9, 11])
        
        # E2
        new_c = self.c.rebinned(step=2, fixp=-5)
        assert np.array_equal(new_c.x, [-1, 1, 3, 5, 7, 9])

And in Curve.rebinned we can find following comment:

        fixp doesn't have to be inside original domain.

As marked in code above #E1 and #E2 throw following exception:

...
>           raise ValueError('in change_domain():' 'the old domain does not include the new one')
E           ValueError: in change_domain():the old domain does not include the new one

beprof/curve.py:113: ValueError

If I comment out raise ValueError() in Curve.change_domain() tests work, no exceptions are thrown... And the above only happens when fixp is outside original domain.

@antnieszka antnieszka added this to the 0.1.1 milestone Dec 5, 2016
@antnieszka antnieszka changed the title rebinned() errors when fixp is outside domain Curve.rebinned() errors when fixp is outside domain Dec 5, 2016
@antnieszka
Copy link
Contributor Author

Another interesting thing:

Python3

        new_c = self.c.rebinned(step=2, fixp=15)
>       assert np.array_equal(new_c.x, [1, 3, 5, 7, 9, 11])
E       AssertionError: assert False
E        +  where False = <function array_equal at 0x7fce54ce9a60>(DataSet([ 1.,  3.,  5.,  7.,  9.]), [1, 3, 5, 7, 9, 11])
E        +    where <function array_equal at 0x7fce54ce9a60> = np.array_equal
E        +    and   DataSet([ 1.,  3.,  5.,  7.,  9.]) = Curve([[ 1.,  1.],\n       [ 3.,  3.],\n       [ 5.,  5.],\n       [ 7.,  3.],\n       [ 9.,  1.]]).x

for it passes... but when I change values in assert, to pass in Python3:

Python2

        new_c = self.c.rebinned(step=2, fixp=15)
>       assert np.array_equal(new_c.x, [1, 3, 5, 7, 9])
E       AssertionError: assert False
E        +  where False = <function array_equal at 0x7f19ce1cb140>(DataSet([  1.,   3.,   5.,   7.,   9.,  11.]), [1, 3, 5, 7, 9])
E        +    where <function array_equal at 0x7f19ce1cb140> = np.array_equal
E        +    and   DataSet([  1.,   3.,   5.,   7.,   9.,  11.]) = Curve([[  1.,   1.],\n       [  3.,   3.],\n       [  5.,   5.],\n       [  7.,   3.],\n       [  9.,   1.],\n       [ 11.,   0.]]).x

tests/test_curve.py:146: AssertionError

@grzanka
Copy link
Collaborator

grzanka commented Dec 5, 2016

It took me a while to figure out what is fixp and step, which means that the docstring of this method has to be improved.

Do not blindly comment out stuff, its programming, not magic.

The comment is correct - rebinned should work also with fixp outstide the domain.

The tasks to be done:

  • figure out what it should really do, see Implement rebinning methods #4 (this is an opportunity to add some reasonable comments)
  • check if it does what is suppose to do
  • fix if necessary

Sorry, something went wrong.

@antnieszka antnieszka modified the milestones: 0.1.0, Next release Dec 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants