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
EnumEnforcerin 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
Enums and having automatic usage enforcement. However, when using aClientin an in interactive console, I found it a bit annoying to have to keep switching between the official API docs and thetda-apidocs to locate the correctEnums 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_valueswhich, when set toTrue, causesEnumEnforcerto allow the direct passage of enumerated values, as long as they are given as definitions in the relevantEnumsubclass. It essentially turnsEnumEnforcerinto 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
EnumEnforcerclass 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(aValueErrorsubclass) andEnumRequiredException(which inherits from bothTypeErrorandValueErrorfor backwards compatibility).EnumRequiredExceptionis raised byEnumEnforcerwhen an invalid value is passed andenforce_enumsisTruebutallow_valuesisFalse.UnrecognizedValueExceptionis raised when an invalid value is passed while both are set toTrue.There was also an issue with the
ValueErrorEnumEnforcerraised--the description said to setenforce_enumstoTrueto 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_iterablewas also refactored to use repeated calls toconvert_enuminstead of implementing its own logic. I noticed thatconvert_enum_iterablewas 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 ofstris 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!