-
-
Notifications
You must be signed in to change notification settings - Fork 7k
only allow integer field to accept integers and strings, fixes #2835 #2836
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
Conversation
I can't say I'm thrilled at the idea of doing type checking here, but I will point out that in Python 2 you also have to be able to handle Django itself handles this by calling |
Thanks Kevin, good call. I adjusted and squashed the change. |
…s being stored as ints, fixes encode#2835. Match IntegerField validation with Django IntegerField, prevents decimal values being stored as ints, fixes encode#2835 On branch master
LGTM |
only allow integer field to accept integers and strings, fixes #2835
Looks good, thanks for making the change. |
@@ -682,7 +682,7 @@ def to_internal_value(self, data): | |||
self.fail('max_string_length') | |||
|
|||
try: | |||
data = int(data) | |||
data = int(re.compile(r'\.0*\s*$').sub('', str(data))) |
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.
- We should push this to
self.re_decimal
as per in Django so that it's only compiled once. - We should comment that this regex substitution is so that we allow eg '1.0' as an int, but not '1.2'
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.
Coupla inline comments that would be good to resolve. |
The following items had a wrong href value: - Dont require pk strictly for related fields. (encode#2745, encode#2754) - Restrict integer field to integers and strings. (encode#2835, encode#2836)
No description provided.