Pe/fix #36
Pe/fix #36
Conversation
Co-authored-by: Max Cohen <[email protected]>
* Added a period parameter to the transformer * Fix Co-authored-by: Max Cohen <[email protected]>
|
There was an error that this fixes @maxjcohen |
|
Hi, if you're talking about the default value of |
|
I'll update it to keep the default period. The actual error is caused here: https://github.com/maxjcohen/transformer/blob/master/tst/transformer.py#L129 so I'll come up with another solution and update this PR accordingly |
|
I see, I'll let you work on it, and see if I can find a alternative later this week. |
|
Give me a day or 2 if you can and I'll have something |
|
@maxjcohen check the update. This should work now |
* Added a period parameter to the transformer * Fix * Fixed Co-authored-by: Max Cohen <[email protected]>
* Added a period parameter to the transformer * Fix * Fixed * Edge check Co-authored-by: Max Cohen <[email protected]>
* Added a period parameter to the transformer * Fix * Fixed * Edge check Co-authored-by: Max Cohen <[email protected]>
| Must be one of `original, `regular` or `None. Default is `None`. | ||
| pe_period: | ||
| If using the ``'regular'` pe, then we can define the period. Default is ``24``. | ||
| If using the ``regular`` pe, then we can define the period. Default is ``24``. |
There was a problem hiding this comment.
The correct syntax here should be:
``'original'``for string values``24``for number values``None``for None
| @@ -64,7 +64,7 @@ def __init__(self, | |||
| dropout: float = 0.3, | |||
| chunk_mode: str = 'chunk', | |||
| pe: str = None, | |||
| pe_period: int = 24): | |||
| pe_period: int = None): | |||
There was a problem hiding this comment.
I prefer having a default value here, of 24.
|
|
||
| if pe == 'regular' and pe_period is not None: | ||
| self._pe_period = pe_period |
There was a problem hiding this comment.
This seems unnecessary, what edge case does it address ?
No description provided.