Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
High-Level API #970
High-Level API #970
Changes from 94 commits
42fc181
a54aade
16ed5fd
25c6bbd
2a1cc6b
316eb3c
997b520
adc3240
d26b8cb
8ec4200
3fd60f9
4d53d34
37dc07e
367778d
6a73938
e993425
38cf982
d4e604b
5bcf514
78b6dd1
acd89fa
cd79cf8
e0e7349
6b6d9ea
2671580
de70147
ce26e25
58bd20f
9f0a410
8f67c2e
358978c
1cba589
b54fcd1
50ac385
d269063
837ff13
a8dc75f
1243894
7ed6c1d
e671632
a161a9c
22dfc4e
4e93c12
6bb3abb
1bb52a6
a8ea680
73a6d15
383a4a6
7af836b
17ef4dd
305b30a
799beb7
c7d0b6b
213e08a
a8a367c
686fd55
ee3813b
f6d4977
3691ed2
ba80329
76e8702
023b33c
3bba192
fc695a5
90eaacb
97e21b5
4b270ea
8304878
ae48506
d84e936
e63d8d4
ff451f8
c7d0cbb
80b1b1f
ed06ab7
41bd463
9c5ee55
cc6f016
193be9a
bbfad01
89ce40e
6cbee18
7437131
f7f2064
b5a8915
58466eb
dd4a0eb
96298ea
da2194e
3cd6dcc
d684dae
c613557
86cca8f
a3dbe90
5952993
fdb0eba
5c8d57a
7e6d3d6
ac672f6
dae4000
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be better to use
kwargs
explicitly in most methods that have more than say 3-4 parameters. Makes it easier to understand the logic and prevents difficult-to-debug mistakes when the order is accidentally switched by the user. You could use(*, <args>)
in the constructor ofPolicyWrapperFactoryIntrinsicCuriosity
and other places where it's appropriate to ensure that this is happening - similar to how I've done it for the policies in the last PRThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, how about using an alias type
IntrinsicCuriousity
for better readability?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, forced kwargs can improve readability (and potentially avoid errors) here. Added in commit da2194e.
As far as the shorter aliases are concerned, I am still not entirely convinced they are needed. I would never use them personally, because they're just not easily discovered via auto-completion. And you lose the semantics. What do the others think?