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

Make some document properties extensible #1163

Merged

Conversation

danielmitterdorfer
Copy link
Member

The Documents domain class currently assumes static corpora and thus
populates all properties eagerly via instance fields. There are cases
however when we want to resolve these attributes lazily because the
respective set of documents is generated at a later stage in the
benchmark and we cannot derive these properties upfront. With this
commit we make the relevant properties extensible so they can be
resolved lazily in subclass implementations.

The `Documents` domain class currently assumes static corpora and thus
populates all properties eagerly via instance fields. There are cases
however when we want to resolve these attributes lazily because the
respective set of documents is generated at a later stage in the
benchmark and we cannot derive these properties upfront. With this
commit we make the relevant properties extensible so they can be
resolved lazily in subclass implementations.
@danielmitterdorfer danielmitterdorfer added enhancement Improves the status quo :Track Management New operations, changes in the track format, track download changes and the like labels Jan 28, 2021
@danielmitterdorfer danielmitterdorfer added this to the 2.0.4 milestone Jan 28, 2021
@danielmitterdorfer danielmitterdorfer self-assigned this Jan 28, 2021
@danielmitterdorfer
Copy link
Member Author

Note to reviewers: I was pondering to expose all instance fields via properties but opted against it because it would make the implementation of this class much larger for little value (except consistency).

@@ -227,6 +227,30 @@ def has_compressed_corpus(self):
def has_uncompressed_corpus(self):
return self.document_file is not None

@property
Copy link
Contributor

Choose a reason for hiding this comment

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

TIL

Copy link
Contributor

@DJRickyB DJRickyB left a comment

Choose a reason for hiding this comment

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

LGTM

@danielmitterdorfer danielmitterdorfer merged commit bbb5ca1 into elastic:master Jan 28, 2021
@danielmitterdorfer danielmitterdorfer deleted the doc-properties branch January 28, 2021 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves the status quo :Track Management New operations, changes in the track format, track download changes and the like
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants