Skip to content

Add rich comparison operators to FunctionxD#324

Merged
CnlPepper merged 4 commits intoraysect:developmentfrom
jacklovell:feature/function-richcmp
Dec 10, 2019
Merged

Add rich comparison operators to FunctionxD#324
CnlPepper merged 4 commits intoraysect:developmentfrom
jacklovell:feature/function-richcmp

Conversation

@jacklovell
Copy link
Contributor

Enable arithmetic comparison operations between FunctionxD objects,
which return 1.0 if the comparison returns True, or 0.0 if the
comparison returns false.

At the moment I've only implemented the 2D operators, so we can discuss the implementation before I go ahead and add the 1D and 3D ones. The pull request would end up being too large to properly review otherwise.

Also, should these features still be based off the feature/function branch? Or is the function framework sufficiently stable that we can add new functions straight to the development branch?

@CnlPepper
Copy link
Member

Thanks Jack, I only just noticed the email about this merge request. I'll have look through asap.

@jacklovell
Copy link
Contributor Author

No worries. I won't be doing any more work on this until #326 is merged, then I'll rebase off of that. I really only want to know whether you agree with the approach taken here.

@jacklovell jacklovell force-pushed the feature/function-richcmp branch from 6b7b298 to 8ac9175 Compare November 29, 2019 17:05
@jacklovell
Copy link
Contributor Author

@CnlPepper I've updated this PR with the new wrapping behaviour, and also addressed all the other comments I remember from our discussion on Wednesday. This is now ready for a preliminary review, and if you're happy with it then I'll add the 1D and 3D cases.

@CnlPepper
Copy link
Member

Hi Jack, sorry for the delay. I'll have a look through later this afternoon/evening if I can.

@jacklovell jacklovell force-pushed the feature/function-richcmp branch from 8ac9175 to 2a2f7e7 Compare December 3, 2019 14:57
@jacklovell
Copy link
Contributor Author

No worries. I've just rebased this onto the latest development (with sqrt and erf), hence the force push. No other changes yet.

Copy link
Member

@CnlPepper CnlPepper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few more tweaks, happy to commit once they are done.

Enable arithmetic comparison operations between FunctionxD objects and
python callables, which return 1.0 if the comparison returns True, or
0.0 if the comparison returns false.
@jacklovell jacklovell force-pushed the feature/function-richcmp branch from 2a2f7e7 to 12f101e Compare December 4, 2019 14:39
This enables taking the absolute value of a Function2D object. The
Function2D/Function2D rich comparison unit test has been modified to
take advantage of this.
@CnlPepper CnlPepper marked this pull request as ready for review December 7, 2019 22:39
@CnlPepper
Copy link
Member

CnlPepper commented Dec 7, 2019

Are you planning to add the 1D and 3D implementations in this merge request or will these follow in a separate request?

@jacklovell
Copy link
Contributor Author

I was planning to add the 1D and 3D implementations in this merge request on Monday morning. But if you want to merge this and add them yourself before then don't let me stop you!

@CnlPepper
Copy link
Member

He he, oh I'm happy to wait for Monday. :)

@jacklovell
Copy link
Contributor Author

@CnlPepper I've added the 1D and 3D cases. This is now ready for review and merge.

@CnlPepper CnlPepper merged commit 9254751 into raysect:development Dec 10, 2019
@CnlPepper
Copy link
Member

Thanks @jacklovell

@jacklovell jacklovell deleted the feature/function-richcmp branch December 10, 2019 14:21
mkgessen pushed a commit to mkgessen/raysect that referenced this pull request Jun 1, 2021
Add rich comparison operators to FunctionxD
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants