Add model's "additionalProperties" handling in python client

[mtngld]

Fix #2037 by adding dictionary handling in a model's to_dict() method

[wing328]

@mtngld thanks for the PR. I notice the commits are not linked to your account, which means this PR won't count as your contribution towards https://github.com/swagger-api/swagger-codegen/graphs/contributors. If you're ok with that, we'll review and merge the PR later.

[wing328]

Also not sure how hard it's but it would be nice to add a test case in the Python Petstore sample to cover the bug moving forward.

Ref: https://github.com/swagger-api/swagger-codegen/blob/master/samples/client/petstore/python/tests/test_pet_model.py

[mtngld]

Hi, hopefully I fixed my account linking issue,

I have no problem adding a testcase, but currently petstore.json does not have a model defintion with additionalProperties, I can add one, but that might affect more modules...

[wing328]

You're right. I'll add a fake model later and would need your help to add a test case if you don't mind.

I think you need to close this PR and open a new one. You can then check the "Commits" tab to confirm.

[wing328]

I can see that it's fixed in the commit tab. will review and approve accordingly.

[wing328]

@mtngld is the change compatible with python3.x?

I ran the integration test as follows but got errors:

cd samples/client/petstore/python
virtualenv venv
source ./venv/bin/activate
./test-all.sh

Let me know if you can't repeat the issue locally.

[wing328]

and here is part of the error message if it helps:

  File "/private/var/tmp/pr/mtngld/swagger-codegen/samples/client/petstore/python/.tox/py34/lib/python3.4/imp.py", line 171, in load_source
    module = methods.load()
  File "<frozen importlib._bootstrap>", line 1220, in load
  File "<frozen importlib._bootstrap>", line 1200, in _load_unlocked
  File "<frozen importlib._bootstrap>", line 1129, in _exec
  File "<frozen importlib._bootstrap>", line 1471, in exec_module
  File "<frozen importlib._bootstrap>", line 321, in _call_with_frames_removed
  File "/private/var/tmp/pr/mtngld/swagger-codegen/samples/client/petstore/python/tests/test_store_api.py", line 14, in <module>
    import swagger_client
  File "/private/var/tmp/pr/mtngld/swagger-codegen/samples/client/petstore/python/swagger_client/__init__.py", line 4, in <module>
    from .models.user import User
  File "/private/var/tmp/pr/mtngld/swagger-codegen/samples/client/petstore/python/swagger_client/models/__init__.py", line 4, in <module>
    from .user import User
  File "/private/var/tmp/pr/mtngld/swagger-codegen/samples/client/petstore/python/swagger_client/models/user.py", line 261
    lambda (k, v): (k, v.to_dict()) if hasattr(v, "to_dict") else (k, v),
           ^
SyntaxError: invalid syntax

[mtngld]

Sorry about that, only tested on 2.7

Now fixed a small compatibility issue, test_all.sh passed using virtualenv on my machine

[wing328]

@mtngld thanks and PR merged.

I saw

test_500_error (tests.test_api_exception.ApiExceptionTests) ... /private/var/tmp/pr/mtngld/swagger-codegen/samples/client/petstore/python/tests/test_api_exception.py:60: DeprecationWarning: Please use assertRaisesRegex instead.
  with self.assertRaisesRegexp(ApiException, "Internal Server Error"):
/private/var/tmp/pr/mtngld/swagger-codegen/samples/client/petstore/python/tests/test_api_exception.py:76: DeprecationWarning: Please use assertRegex instead.

I will create a task to track it and see if anyone from the community has cycle to update the code with assertRegex instead.

[wing328]

@mtngld btw, one suggestion is to create a new branch instead fixing the issue in the master (I believe creating a new branch for change is part of the Git best practices)

[mtngld]

Hi, @wing328, sorry for all the bother I caused, but I found a bug in my PR, I made a new branch that:

1. Fix the bug I found (errrr python2.7 and python3.4 issues...)
2. Performance optimization by switching the order of the elif, since I guess additionalProperties is probably less used and should be checked last
3. Style optimization by warping lines and staying below 79 chars
4. Change value.iteritems() to value.items()

Should I create a new PR?

The new branch is based on the current master (which includes my previous PR), let me know if you want to simply revert the old PR and I will rebase to a clean and updated master.

Sorry again and thank you for your patience and guidance

We should definetly write test case for this scenario since that bug was not caught by test-all.sh but was found when using my own swagger file.

[wing328]

Please submit a PR for the fix.

William

On Mon, Feb 8, 2016 at 3:56 AM, Matan Goldman notifications@github.com
wrote:

Hi, @wing328 https://github.com/wing328, sorry for all the bother I
caused, but I found a bug in my PR, I made a new branch
https://github.com/mtngld/swagger-codegen/tree/issue_2037 that:

1. Fix the bug I found (errrr python2.7 and python3.4 issues...)
2. Performance optimization by switching the order of the elif, since
   I guess additionalProperties is probably less used and should be
   checked last
3. Style optimization by warping lines and staying below 79 chars
4. Change value.iteritems() to value.items()

Should I create a new PR?

The new branch is based on the current master (which includes my previous
PR), let me know if you want to simply revert the old PR and I will rebase
to a clean and updated master.

Sorry again and thank you for your patience and guidance

We should definetly write test case for this scenario since that bug
was not caught by test-all.sh but was found when using my own swagger
file.

—
Reply to this email directly or view it on GitHub
#2058 (comment)
.