Optionally Allow Direct Passing of Values with EnumEnforcer #170
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.
I decided to make a change to
EnumEnforcer
in the fork of this repository that I have been using, and figured I'd clean it up, add tests, and run it by you guys. Please consider this PR as a suggestion--I will not be offended if you'd rather not implement this feature, especially given that I did not consult with anyone about this change. Feel free to "take it or leave it".I fully agree with the rationale behind storing the allowed API request parameters in
Enum
s and having automatic usage enforcement. However, when using aClient
in an in interactive console, I found it a bit annoying to have to keep switching between the official API docs and thetda-api
docs to locate the correctEnum
s to use for every request parameter. The imports are also very verbose, and generally not very conducive to interactive usage.As a result, I propose the following compromise: optionally allow direct passing of values using a new keyword for
EnumEnforcer
(andBaseClient
) calledallow_values
which, when set toTrue
, causesEnumEnforcer
to allow the direct passage of enumerated values, as long as they are given as definitions in the relevantEnum
subclass. It essentially turnsEnumEnforcer
into a spell-checker. I believe I have managed to implement this feature without breaking backwards compatibility, with the possible exception ofEnumEnforcer.type_error
(I'm not sure if this method was considered to be public facing--it was not documented) which, interestingly, had the effect of raising aValueError
.I also noticed that the
EnumEnforcer
class did not have unit tests, so I went ahead and wrote them (accounting for this new keyword argument as well as original use cases). I added some new exceptions calledUnrecognizedValueException
(aValueError
subclass) andEnumRequiredException
(which inherits from bothTypeError
andValueError
for backwards compatibility).EnumRequiredException
is raised byEnumEnforcer
when an invalid value is passed andenforce_enums
isTrue
butallow_values
isFalse
.UnrecognizedValueException
is raised when an invalid value is passed while both are set toTrue
.There was also an issue with the
ValueError
EnumEnforcer
raised--the description said to setenforce_enums
toTrue
to disable the parameter checking. I assume this was unintentional. I went ahead and changed the error messages to explain the two keywords and also link to this repository in case a valid value is missing from anEnum
.EnumEnforcer.convert_enum_iterable
was also refactored to use repeated calls toconvert_enum
instead of implementing its own logic. I noticed thatconvert_enum_iterable
was written such that it was acceptable to pass a single enum member, which would be converted to a list of one element. I kept this functionality for backwards compatibility, but only for when an enum member is passed--not when a value is passed. This is because an instance ofstr
is both a valid value for some enums but also an iterable, so it makes more sense to just force the user to use a sequence of one element instead in this case.Please let me know if this is of interest to the maintainers/community of this repo. Cheers!