CSHARP-5527: Support $sigmoid expression #1638
Conversation
| if (method.Is(MongoDBMathMethod.Sigmoid)) | ||
| { | ||
| var argExpression = arguments.Single(); | ||
| var argTranslation = ExpressionToAggregationExpressionTranslator.Translate(context, argExpression); |
There was a problem hiding this comment.
Should we check if argTranslation has numeric bson representaiton?
|
|
||
| namespace MongoDB.Driver.Tests.Linq.Linq3Implementation.Jira | ||
| { | ||
| public class CSharp5527Tests : LinqIntegrationTest<CSharp5527Tests.ClassFixture> |
There was a problem hiding this comment.
I would suggest to rename this test class to SigmoidMethodToAggregationExpressionTranslatorTests and move into appropriate namespace.
| [Fact] | ||
| public void Sigmoid_should_work() | ||
| { | ||
| RequireServer.Check().Supports(Feature.Sigmoid); |
There was a problem hiding this comment.
Move this condition to ctor to avoid collection creation in case server does not support the feature. Like here:
| /// <summary> | ||
| /// Gets the $sigmoid operator feature. | ||
| /// </summary> | ||
| public static Feature Sigmoid => __sigmoid; |
There was a problem hiding this comment.
Minor: rename feature to SigmoidOperator.
| if (method.Is(MongoDBMathMethod.Sigmoid)) | ||
| { | ||
| var argExpression = arguments.Single(); | ||
| var argTranslation = ExpressionToAggregationExpressionTranslator.Translate(context, argExpression); |
| if (method.Is(MongoDBMathMethod.Sigmoid)) | ||
| { | ||
| var argExpression = arguments.Single(); | ||
| var argTranslation = ExpressionToAggregationExpressionTranslator.Translate(context, argExpression); |
There was a problem hiding this comment.
Minor: keep these names consistent with the declaration of the method, so rename to valueExpression and valueTranslation.
| /// </summary> | ||
| /// <param name="value">The input value.</param> | ||
| /// <returns>The transformed value.</returns> | ||
| public static double Sigmoid(double value) |
There was a problem hiding this comment.
While this is a perfectly reasonable place to declare this method we should discuss a bit whether going forward we want all new methods like this to live in the Mql class.
No description provided.