Skip to content

Fix to factorial.discrete so that it doesn't require custom init_prepare#244

Merged
Sahel13 merged 3 commits into
mainfrom
factorial-discrete-test
Jun 2, 2026
Merged

Fix to factorial.discrete so that it doesn't require custom init_prepare#244
Sahel13 merged 3 commits into
mainfrom
factorial-discrete-test

Conversation

@SamDuffield
Copy link
Copy Markdown
Contributor

The current code for factorial.discrete is insufficient as it requires overwriting the init_prepare in discrete.filter. This isn't ideal as factorial code is supposed to work with the cuthbert filters without modification.

I'm wondering about a more holistic fix. Maybe we can replace Factorializer with FactorialFilter which still has the extract, join functions etc but also has init_prepare, filter_prepare, filter_combine which are just inherited from cuthbert or wrapped in the case of init_prepare. Although this may create too much code clutter. Will think about it some more

@SamDuffield SamDuffield changed the title Fix to factorial.discrete so that it doesn't custom init_prepare Fix to factorial.discrete so that it doesn't custom init_prepare Jun 2, 2026
@Sahel13
Copy link
Copy Markdown
Collaborator

Sahel13 commented Jun 2, 2026

I'm wondering about a more holistic fix. Maybe we can replace Factorializer with FactorialFilter which still has the extract, join functions etc but also has init_prepare, filter_prepare, filter_combine which are just inherited from cuthbert or wrapped in the case of init_prepare. Although this may create too much code clutter. Will think about it some more

This is related to #245

@SamDuffield SamDuffield changed the title Fix to factorial.discrete so that it doesn't custom init_prepare Fix to factorial.discrete so that it doesn't require custom init_prepare Jun 2, 2026
@SamDuffield SamDuffield requested a review from Sahel13 June 2, 2026 20:53
@SamDuffield SamDuffield marked this pull request as ready for review June 2, 2026 20:53
@SamDuffield
Copy link
Copy Markdown
Contributor Author

Fixed with minor modification to cuthbert.discrete.filter doesn't change anything for the user there though

@Sahel13 Sahel13 merged commit 2e360de into main Jun 2, 2026
2 checks passed
@Sahel13 Sahel13 deleted the factorial-discrete-test branch June 2, 2026 21:36
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.

2 participants