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

Added CWTOutput class to fix the issues with the new main cwt method.… #21

Merged
merged 4 commits into from
Jul 28, 2022

Conversation

avcarr2
Copy link
Owner

@avcarr2 avcarr2 commented Jul 26, 2022

… Then I fixed all the tests

  • Removed several tests that were either not necessary.
  • Commented out a test that needs updating.
  • Removed a method that was deprecated with the new CWT method.

@avcarr2 avcarr2 requested a review from zdanaceau July 26, 2022 22:23
@avcarr2 avcarr2 linked an issue Jul 26, 2022 that may be closed by this pull request
This was referenced Jul 27, 2022
Copy link
Collaborator

@zdanaceau zdanaceau left a comment

Choose a reason for hiding this comment

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

See the comment on the GetComponent method and fix what I believe is the inefficiency in the CalculateTimeAxis function.

default:
return null;
}
}
private double[,] GetComponent(CWTComponent comp, double[,] originalArray)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the intention for this method to be removed/rendered obsolete?

@@ -86,6 +86,20 @@ public void SplitRealAndImaginary(CWTComponent comp, out double[,]? realCwt, out
break;
}
}
public double[,]? GetComponent(CWTComponent comp, CWTOutput cwtOutput)
Copy link
Collaborator

Choose a reason for hiding this comment

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

If the only function of this method to return Real, Imaginary or Both as as a single array, wouldn't it make more sense to just have a method to get both components in a single array since SplitRealAndImaginary already gets a double[ , ] corresponding to the real and imaginary components?

FCWTAPI/CWTObject.cs Show resolved Hide resolved
{
case 0: return cwtoutput.RealArray.GetLength(0);
case 1: return cwtoutput.RealArray.GetLength(1);
default: return -1;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be better to throw an exception here to make it more traceable if an int != 0 or 1 was entered?

FCWTAPI/CWTObject.cs Show resolved Hide resolved
double testPoint = Math.Atan(imagCwt[32, 32] / realCwt[32, 32]);
Assert.AreEqual(testPoint, testPhase[32, 32], 0.001);
}
//[Test]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you comment this out?

@avcarr2 avcarr2 merged commit b9f1901 into master Jul 28, 2022
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.

CWT Line 12
2 participants