Skip to content
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

C# API client #98

Closed
wants to merge 3 commits into from
Closed

C# API client #98

wants to merge 3 commits into from

Conversation

anguoyang
Copy link

add c# client source code

@anguoyang anguoyang mentioned this pull request Apr 4, 2016
@beniz beniz added the kind:API label Apr 4, 2016
@beniz beniz changed the title Master C# API client Apr 9, 2016
@beniz
Copy link
Collaborator

beniz commented Apr 9, 2016

Hi @anguoyang thanks for contributing this first version of the client!

I have several remarks that needs to be addressed before this code can make it into deepdetect. I would definitely understand if you do not have the time required to make the changes.

Remarks are below:

  • It is best if the client has the ability to build the API calls programmatically. In the present case of your C# client, this means that the client should take JObject for parameter_input,parameters_output and parameters_mllib. See the Python client for an example of how to do this: https://github.com/beniz/deepdetect/blob/master/clients/python/dd_client.py#L274
  • It is more practical if the client defines the API functions, much like in the Python client, i.e. info(),predict(...),train(...), etc.
  • The HTTP connection code must have error handling, there's no way to avoid this otherwise the client would not be ready for production.
  • I don't see where your functions probs(), problast, etc... are used ?

Again, thanks for this first version, let me know if you believe you'd be able to provide changes to accomodate some of the points above!

@anguoyang
Copy link
Author

Hi, @beniz ,

1.It is best if the client has the ability to build the API calls programmatically. In the present case of your C# client, this means that the client should take JObject for parameter_input,parameters_output and parameters_mllib. See the Python client for an example of how to do this: https://github.com/beniz/deepdetect/blob/master/clients/python/dd_client.py#L274

A: Yes, it is a good idea to make the API calls programmatically, I will add it recently. As for JObject, it is only the output/result string object.

2.It is more practical if the client defines the API functions, much like in the Python client, i.e. info(),predict(...),train(...), etc.

A:Ok, I will learn from it, although I am not familiar with python, it is not difficult.

3.The HTTP connection code must have error handling, there's no way to avoid this otherwise the client would not be ready for production.

A:Yes, I agree with you, I need to add try catch for it.

4.I don't see where your functions probs(), problast, etc... are used ?

A: I have not used these functions, in this first version, I only used the "predict", but util now, I have already added "service creation -put_service", "service deletion" and with some other update, and in the next days will add train, and hopefully add the "getting current services", for probs() or problast, I
could not find any related information in python client, is it the items in prediction result string? I saw it in the JObject, and tried to get probs and cat with my method in C# like this, with success:

        public static string GetMiddle(string full, string start, string end)
        {
            int IndexofA = full.IndexOf(start);
            int IndexofB = full.IndexOf(end);
            return full.Substring(IndexofA + start.Length, IndexofB - IndexofA - start.Length);
        }
            prediction = GetMiddle(result, "\"prob\": ", ",\r\n");
            cat = GetMiddle(result, "\"cat\": ", "\"\r\n}");
            cat = cat.Substring(cat.IndexOf(" ") + 1);

It will not take so much time for me on C# client, so, please don't worry about it:), actually, I am happy to be a member of deepdetect.

@beniz
Copy link
Collaborator

beniz commented Apr 11, 2016

hi @anguoyang thanks for taking the time and energy for this!

A: Yes, it is a good idea to make the API calls programmatically, I will add it recently. As for JObject, it is only the output/result string object.

You may have to use it as input as well. If users can programmatically build a JObject for each of the section of the API objects (input,mllib and output in parameters object), this would allow the client to not depend on updates and changes in the API.

could not find any related information in python client, is it the items in prediction result string?

Yes, at the moment the return to predict from the Python client is a JSON object. However, there's plan to do something similar as what you are doing at the moment, i.e. exposing the categories and probabilities directly. However, this is optional at the moment, the proper way of doing is to return the full JSON object to the predict call. One reason is that categories and probabilities only apply to the supervised case, whereas in the unsupervised case, output is a tensor of some dimension.

Don't hesitate if you have questions and / or need help on any of the related topics.

@skynode
Copy link

skynode commented Jun 5, 2017

Hi @anguoyang ,

Have you completed this C# port?

@beniz beniz closed this Jan 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants