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.

Sign up for GitHub

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

Jump to bottom

Allow inheriting the Transformer class #2810

Merged
merged 1 commit into from
Jul 5, 2024
Merged

Allow inheriting the Transformer class #2810

merged 1 commit into from
Jul 5, 2024

Conversation

mokha
Copy link
Contributor

@mokha mokha commented Jul 4, 2024

Creating a custom Transformer class that inherits Transformer would fail as the load function would always the original class. This change would allow smooth inheritance

tomaarsen
tomaarsen approved these changes Jul 5, 2024
View reviewed changes
Copy link
Collaborator

@tomaarsen tomaarsen left a comment

Choose a reason for hiding this comment

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

I agree completely, this is a much better convention for loading a class instance.

@tomaarsen tomaarsen merged commit 0ff4e0c into UKPLab:master Jul 5, 2024
11 checks passed
fpgmaas pushed a commit to fpgmaas/sentence-transformers that referenced this pull request Jul 9, 2024
@tomaarsen @fpgmaas
Update outdated docs links
23dfa17 
Allow inheriting the Transformer class (UKPLab#2810)

[`feat`] Add hard negatives mining utility (UKPLab#2768)

* Add hard negatives mining utility

* Add example datasets/models for hard negative mining tip

* Update phrasing in dataset overview

[chore] add test for NoDuplicatesBatchSampler (UKPLab#2795)

* add test for NoDuplicatesBatchSampler

* formatting

* simplify tests

[chore] Add test for RoundrobinBatchSampler (UKPLab#2798)

* Add test for RoundrobinBatchSampler

* fix test

* improve RoundRobinBatchSampler and add additional test

* Make datasets in ConcatDataset different sizes

As the real "use case" of the RoundRobin sampler is to avoid sampling from one dataset more than from another. This is best tested when the datasets have different sizes.

---------

Co-authored-by: Tom Aarsen <[email protected]>

[feat] Improve GroupByLabelBatchSampler (UKPLab#2788)

* Improve GroupByLabelBatchSampler

* small fix

* improve test

* Update sentence_transformers/sampler.py

Co-authored-by: Tom Aarsen <[email protected]>

* fix sampler and add unit test

* fix comment

* remove .DS_Store

* rm DS_Store

* change self.groups statement

* move to damplers dir

* Update sentence_transformers/sampler.py

Co-authored-by: Tom Aarsen <[email protected]>

* Add typing

---------

Co-authored-by: Tom Aarsen <[email protected]>
Co-authored-by: Tom Aarsen <[email protected]>

[`chore`] Clean-up `.gitignore` (UKPLab#2799)

add test coverage command

add to workflow

fix cicd

fix cicd

fix

leave cicd untouched

fix gitignore

fix gitignore

update gitignore

update gitignore

fix gitignore

fix gitignor
fpgmaas pushed a commit to fpgmaas/sentence-transformers that referenced this pull request Jul 9, 2024
@tomaarsen @fpgmaas
Update outdated docs links
30064e2 
Allow inheriting the Transformer class (UKPLab#2810)

[`feat`] Add hard negatives mining utility (UKPLab#2768)

* Add hard negatives mining utility

* Add example datasets/models for hard negative mining tip

* Update phrasing in dataset overview

[chore] add test for NoDuplicatesBatchSampler (UKPLab#2795)

* add test for NoDuplicatesBatchSampler

* formatting

* simplify tests

[chore] Add test for RoundrobinBatchSampler (UKPLab#2798)

* Add test for RoundrobinBatchSampler

* fix test

* improve RoundRobinBatchSampler and add additional test

* Make datasets in ConcatDataset different sizes

As the real "use case" of the RoundRobin sampler is to avoid sampling from one dataset more than from another. This is best tested when the datasets have different sizes.

---------

Co-authored-by: Tom Aarsen <[email protected]>

[feat] Improve GroupByLabelBatchSampler (UKPLab#2788)

* Improve GroupByLabelBatchSampler

* small fix

* improve test

* Update sentence_transformers/sampler.py

Co-authored-by: Tom Aarsen <[email protected]>

* fix sampler and add unit test

* fix comment

* remove .DS_Store

* rm DS_Store

* change self.groups statement

* move to damplers dir

* Update sentence_transformers/sampler.py

Co-authored-by: Tom Aarsen <[email protected]>

* Add typing

---------

Co-authored-by: Tom Aarsen <[email protected]>
Co-authored-by: Tom Aarsen <[email protected]>

[`chore`] Clean-up `.gitignore` (UKPLab#2799)

add test coverage command

add to workflow

fix cicd

fix cicd

fix

leave cicd untouched

fix gitignore

fix gitignore

update gitignore

update gitignore

fix gitignore

fix gitignor
fpgmaas pushed a commit to fpgmaas/sentence-transformers that referenced this pull request Jul 9, 2024
@tomaarsen @fpgmaas
Update outdated docs links
7976a06 
Allow inheriting the Transformer class (UKPLab#2810)

[`feat`] Add hard negatives mining utility (UKPLab#2768)

* Add hard negatives mining utility

* Add example datasets/models for hard negative mining tip

* Update phrasing in dataset overview

[chore] add test for NoDuplicatesBatchSampler (UKPLab#2795)

* add test for NoDuplicatesBatchSampler

* formatting

* simplify tests

[chore] Add test for RoundrobinBatchSampler (UKPLab#2798)

* Add test for RoundrobinBatchSampler

* fix test

* improve RoundRobinBatchSampler and add additional test

* Make datasets in ConcatDataset different sizes

As the real "use case" of the RoundRobin sampler is to avoid sampling from one dataset more than from another. This is best tested when the datasets have different sizes.

---------

Co-authored-by: Tom Aarsen <[email protected]>

[feat] Improve GroupByLabelBatchSampler (UKPLab#2788)

* Improve GroupByLabelBatchSampler

* small fix

* improve test

* Update sentence_transformers/sampler.py

Co-authored-by: Tom Aarsen <[email protected]>

* fix sampler and add unit test

* fix comment

* remove .DS_Store

* rm DS_Store

* change self.groups statement

* move to damplers dir

* Update sentence_transformers/sampler.py

Co-authored-by: Tom Aarsen <[email protected]>

* Add typing

---------

Co-authored-by: Tom Aarsen <[email protected]>
Co-authored-by: Tom Aarsen <[email protected]>

[`chore`] Clean-up `.gitignore` (UKPLab#2799)

add test coverage command

add to workflow

fix cicd

fix cicd

fix

leave cicd untouched

fix gitignore

fix gitignore

update gitignore

update gitignore

fix gitignore

fix gitignor
tomaarsen added a commit that referenced this pull request Jul 9, 2024
@fpgmaas @tomaarsen
[chore] Add pytest-cov and add test coverage command to the Makefile (
5a71df8 
#2794)

* Update outdated docs links

Allow inheriting the Transformer class (#2810)

[`feat`] Add hard negatives mining utility (#2768)

* Add hard negatives mining utility

* Add example datasets/models for hard negative mining tip

* Update phrasing in dataset overview

[chore] add test for NoDuplicatesBatchSampler (#2795)

* add test for NoDuplicatesBatchSampler

* formatting

* simplify tests

[chore] Add test for RoundrobinBatchSampler (#2798)

* Add test for RoundrobinBatchSampler

* fix test

* improve RoundRobinBatchSampler and add additional test

* Make datasets in ConcatDataset different sizes

As the real "use case" of the RoundRobin sampler is to avoid sampling from one dataset more than from another. This is best tested when the datasets have different sizes.

---------

Co-authored-by: Tom Aarsen <[email protected]>

[feat] Improve GroupByLabelBatchSampler (#2788)

* Improve GroupByLabelBatchSampler

* small fix

* improve test

* Update sentence_transformers/sampler.py

Co-authored-by: Tom Aarsen <[email protected]>

* fix sampler and add unit test

* fix comment

* remove .DS_Store

* rm DS_Store

* change self.groups statement

* move to damplers dir

* Update sentence_transformers/sampler.py

Co-authored-by: Tom Aarsen <[email protected]>

* Add typing

---------

Co-authored-by: Tom Aarsen <[email protected]>
Co-authored-by: Tom Aarsen <[email protected]>

[`chore`] Clean-up `.gitignore` (#2799)

add test coverage command

add to workflow

fix cicd

fix cicd

fix

leave cicd untouched

fix gitignore

fix gitignore

update gitignore

update gitignore

fix gitignore

fix gitignor

* add command to open cov

* fix setup.py

* remove open command

---------

Co-authored-by: Tom Aarsen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tomaarsen tomaarsen tomaarsen approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mokha @tomaarsen