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

ENH: raise error if operands of binary operations do not have same "closed" value #96

Closed
venaturum opened this issue Sep 11, 2021 · 4 comments · Fixed by #107
Closed
Assignees
Labels
advanced difficulty level enhancement New feature or request high priority

Comments

@venaturum
Copy link
Collaborator

Describe the solution you'd like

Stairs instances have a "closed" property which can be "left" or "right" indicates if they are left-closed, right-open or right-closed left-open.

When performing binary operations with two Stairs objects, like addition, &, >= etc it is important that both operands have the same value of closed, and an error should be raised if not.

It would be great to have this functionality implemented as a function decorator, which knows to compare self and other parameters which appear in these binary operator methods.

@venaturum venaturum added advanced difficulty level enhancement New feature or request high priority labels Sep 11, 2021
@amagee
Copy link
Contributor

amagee commented Sep 12, 2021

take

@venaturum
Copy link
Collaborator Author

There is a function _sanitise_binary_operands which could come in handy. It is called each time a binary operation is applied, to convert any numerical operands to Stairs.

At the moment when it converts from numerical to Stairs, the default value for "closed" is used ("left"). We may need to set the closed parameter accordingly in this function, and/or make sure the checks pass if Stairs.number_of_steps == 0.

I.e.

sc.Stairs(closed="left") + 1

sc.Stairs(closed="right") + 1

1 + sc.Stairs(closed="left")

1 + sc.Stairs(closed="right")

would all need to work without errors.

@venaturum
Copy link
Collaborator Author

I've suggested it would be nice to have the functionality as a decorator, (and to be honest _sanitise_binary_operands would be nice to have as a decorator), but first step is probably to iron out any wrinkles in the logic, and have it pass tests before we worry about transforming it into a decorator.

I might write some tests for it if that helps?

@venaturum
Copy link
Collaborator Author

@amagee actually, I noticed you're a few steps ahead :D I'll take a look at your PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
advanced difficulty level enhancement New feature or request high priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants