Skip to content

Conversation

@Simon-Prevoteaux
Copy link

Added a classifier class based on the VGG16 model, a test file for the vgg and a comparaison between the vgg model and the googlenet.

@kastnerkyle
Copy link
Contributor

Eesh, I guess the circle CI stuff is still running on PRs... I will try to fix that and review this ASAP.

@Simon-Prevoteaux
Copy link
Author

I have to say that i don't really see how to fix it ... I saw that there was some circle CI code on your last PR. If there is anything I can do, tell me and i'll try to make it work.

@kastnerkyle
Copy link
Contributor

I will fix it and try to rerun the test (I think I can do that) - do you
think it is basically "ready for review" at this point?

On Mon, Apr 18, 2016 at 4:33 PM, Simon notifications@github.com wrote:

I have to say that i don't really see how to fix it ... I saw that there
was some circle CI code on your last PR. If there is anything I can do,
tell me and i'll try to make it work.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub
#99 (comment)

@Simon-Prevoteaux
Copy link
Author

Ok great ! Yes I think it is. I hope i did everything right !

@Simon-Prevoteaux
Copy link
Author

Hi Kyle, I saw that you fixed the CircleCI stuff, so I updated my PR so that it passes the tests. Therefore I think its ok, let me know is there are still problems.

@kastnerkyle
Copy link
Contributor

OK awesome. I am pretty sure it is good to go but I will give it a once
over this morning. This circleCI stuff took more work than I anticipated :)

On Wed, Jul 6, 2016 at 8:36 AM, Simon notifications@github.com wrote:

Hi Kyle, I saw that you fixed the CircleCI stuff, so I updated my PR so
that it passes the tests. Therefore I think its ok, let me know is there
are still problems.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#99 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABfbHd7joKRRdWGfQZCT9uz0uf44-lC0ks5qS8t6gaJpZM4H-f-c
.

@kastnerkyle
Copy link
Contributor

The only thing I see from this glance over is squashing the commits into
one - if you need help with that let me know. It looks really awesome -
thanks for doing this, I know we are kinda slow

On Wed, Jul 6, 2016 at 9:45 AM, Kyle Kastner kastnerkyle@gmail.com wrote:

OK awesome. I am pretty sure it is good to go but I will give it a once
over this morning. This circleCI stuff took more work than I anticipated :)

On Wed, Jul 6, 2016 at 8:36 AM, Simon notifications@github.com wrote:

Hi Kyle, I saw that you fixed the CircleCI stuff, so I updated my PR so
that it passes the tests. Therefore I think its ok, let me know is there
are still problems.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#99 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABfbHd7joKRRdWGfQZCT9uz0uf44-lC0ks5qS8t6gaJpZM4H-f-c
.

… added a test for that, and a comparaison with the googlenet model. Remains a problem with de balanced vgg

fixed a mistake concerning the balanced vgg model

finalizing the code to prepare the PR, and added the vgg classfier to the init file

added a circle.yml file in order to pass the circleCI tests

Update balanced_vgg.py

Added circle.yml from scikit-learn, [doc skip]

Update circle.yml

added the protobuf dependency

fixed a typo
@kastnerkyle
Copy link
Contributor

I can also squash it down (I think) if you want. Just let me know.

@kastnerkyle
Copy link
Contributor

OK - I will try and squash this down and merge in the next few days (if you don't get to it @Simon-Prevoteaux ) it looks pretty good to me!


def test_googlenet_classifier():
"""smoke test for googlenet classifier"""
print 'testing the googlenet classifier'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

py3 should be print( ) but we probably don't need this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants