Skip to content

add _RequireAttrsABCMeta and make the BaseLearner use it - #222

Merged
basnijholt merged 11 commits into
masterfrom
attr_checking_base_class
Oct 17, 2019
Merged

add _RequireAttrsABCMeta and make the BaseLearner use it#222
basnijholt merged 11 commits into
masterfrom
attr_checking_base_class

Conversation

@basnijholt

@basnijholtbasnijholt commented Oct 14, 2019

Copy link
Copy Markdown
Member

This makes sure that the Learners will conform to the default API.

jbweston
jbweston requested changes Oct 16, 2019
View reviewed changes

@jbwestonjbweston left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Other than those changes LGTM

Comment threadadaptive/utils.py Outdated
Comment threadadaptive/learner/base_learner.py
@basnijholt

basnijholt commented Oct 16, 2019

Copy link
Copy Markdown
MemberAuthor

@akhmerov and @jbweston, I've implemented this as we discussed and fixed the issues in the learners that didn't implement these attributes.

@jbweston
jbweston self-requested a review October 17, 2019 06:58
jbweston
jbweston requested changes Oct 17, 2019
View reviewed changes
Comment threadadaptive/utils.py Outdated
Comment threadadaptive/learner/base_learner.py Outdated
jbweston
jbweston requested changes Oct 17, 2019
View reviewed changes
Comment threadadaptive/utils.py Outdated
akhmerov
akhmerov reviewed Oct 17, 2019
View reviewed changes
Comment threadadaptive/utils.py
@basnijholt
basnijholt merged commit ede9582 into masterOct 17, 2019
@basnijholt
basnijholt deleted the attr_checking_base_class branch October 17, 2019 10:12
@basnijholtbasnijholt mentioned this pull request Jan 16, 2020
Closed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jbwestonjbwestonjbweston approved these changes
@akhmerovakhmerovakhmerov 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.

3 participants

@basnijholt@akhmerov@jbweston