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

[BUG] in CNNClassifier and CNNRegressor, ensure filter_sizes and padding is passed on #6452

Merged
merged 2 commits into from
May 27, 2024

Conversation

fkiraly
Copy link
Collaborator

@fkiraly fkiraly commented May 20, 2024

This PR adds filter_sizes and padding parameters to CNNClassifier and CNNRegressor, and ensures these are passed on to CNNNetwork.

Fixes #6436.

@fkiraly fkiraly added module:classification classification module: time series classification module:regression regression module: time series regression bugfix Fixes a known bug or removes unintended behavior labels May 20, 2024
Copy link
Collaborator

@yarnabrina yarnabrina left a comment

Choose a reason for hiding this comment

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

filter_sizes is present in different position than argument order in docstring.

@fkiraly
Copy link
Collaborator Author

fkiraly commented May 20, 2024

Then, should we change the docstring to match actual argument order?

We can add new arguments only at the end, without deprecation.

I changed the docstrings now so the sequence of arguments is in line with the sequence in __init__.

@fkiraly fkiraly merged commit adf99fd into main May 27, 2024
71 checks passed
@fkiraly fkiraly deleted the cnn-params branch May 27, 2024 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes a known bug or removes unintended behavior module:classification classification module: time series classification module:regression regression module: time series regression
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] CNNClassifier and CNNRegressor do not accept filter_sizes argument listed in docstring
2 participants