Source creation from inline data #114
Conversation
| create_args = {} | ||
| if args is not None: | ||
| create_args.update(args) | ||
| if 'source_parser' in create_args: |
There was a problem hiding this comment.
Is there a special reason for this 'source_parser' handling? args is supposed to contain a dictionary with all the nested structure you need to specify all changes. The final json.dumps in line 183 should take care of the JSON string encoding at the end, isn't it?
|
It mostly looks good. Just one more comment. Could you please change the mail address in the commit to your bigml.com one? I believe is easily done with git commit --amend --author or something similar... |
|
OK, I think I managed to fix the e-mail address. I've also added a bit of type validation logic. |
| @@ -86,6 +88,28 @@ def _create_remote_source(self, url, args=None): | |||
| body = json.dumps(create_args) | |||
| return self._create(self.source_url, body) | |||
|
|
|||
| def _create_inline_source(self, src_obj, args=None): | |||
| """Create source from inline data | |||
|
|
|||
There was a problem hiding this comment.
The checking is right, but I miss some explanation in the method's doc saying what's the allowed structure and types for the src_obj arg.
|
Would you mind adding the corresponding explanation in the docs/index.rst document? Just the general use, the type and structure of the src_obj argument and an example would be great. |
|
By the way, I still don't understand the 'source_parser' special handling in line 98... |
|
OK, I see how the source_parser section is redundant. I have removed it. On Tue, Jan 13, 2015 at 11:46 AM, mmerce notifications@github.com wrote:
|
|
Did you upload it? I cannot see the changes... |
|
Oops, I guess I forgot. The changes should be up now. On Tue, Jan 13, 2015 at 2:57 PM, mmerce notifications@github.com wrote:
|
|
Alright, I just ran the test suite and there's a couple of predictions that changed in the clusters, but this has nothing to do with your changes, so this is good news :-). However, before merging we would need you to:
If you need help in this last step, check first the instructions in: https://ariejan.net/2011/07/05/git-squash-your-latests-commits-into-one/ Finally when you see your repo line (you can type gitk in your command line and will see a graphical representation of all commits network) it should appear as a single line, (not as some diverging and rejoining paths). You can ping me for help, of course. |
Signed-off-by: Chee Sing Lee <lee@bigml.com>
|
OK, I think I've got it now. |
Signed-off-by: Chee Sing Lee lee@bigml.com