Conversation
|
|
||
|
|
||
| /* | ||
| T for Transform |
There was a problem hiding this comment.
often, the need to explain is an indication that a name could be improved. If this class is not used dense repetiton, like classes like Array or Event, you could just as well write it out.
TransformNormalizer
This would be better because T may mean a lot of different things (think of task inTdef), and Normalizer is a UGen.
Also possible is to make the verb the name of a class:
NormalizeTransform
Both I find fine.
| } | ||
|
|
||
| init {|dataset| | ||
| if(dataset.isKindOf(SequenceableCollection).not ) { "Dataset must be a Matrix".error;this.halt; }; |
There was a problem hiding this comment.
Better write:
Error("Dataset must be a Matrix".).throw
There was a problem hiding this comment.
btw. the test .isKindOf(SequenceableCollection) doesn't exclude things like a Set. So if you pass a Set it would go through.
|
|
||
| // denormalize a point-slope form 2-dimensional line | ||
| // of the form [slope, intercept] | ||
| denormalizeLine {|line| |
There was a problem hiding this comment.
can you check the formatting of whitespace? Please use tabs not spaces, and auto-formatting.
|
|
||
| T is for Transform | ||
| */ | ||
| TStandardizer { |
There was a problem hiding this comment.
TransformStandardizer or StandardizeTransform?
| /* | ||
| T for Transform | ||
| */ | ||
| TPCA { |
There was a problem hiding this comment.
hm? This class is just an object?
update from main repo
|
One thing of concern I'm spotting is: This change would deprecate the current behavior of I'm not opposed to the use of a more descriptive method name, but, such a change without a deprecation warning could easily break existing code that that uses Having a very quick look on the SC wiki I'm not finding instructions for deprecations. I seem to recall the thing to do is:
Ah... actually, here's a short note: use |
|
I agree that the old method should be deprecated and redirect to the new method. Example of deprecations can be found in asInt {
this.deprecated(thisMethod, this.class.findMethod(\asInteger));
^this.asInteger;
} |
|
Hello @jreus do you intend on addressing review comments in this PR? |
No description provided.