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

Move image gradient extensions to plug-in area #947

Merged
merged 5 commits into from
Sep 26, 2016

Conversation

gheinrich
Copy link
Contributor

This includes several changes:

  • move the data extension to the plug-in area (and wrap it into an installable package)
  • move the view extension to the plug-in area (and wrap it into an installable package)
  • remove config options for extensions
  • update Travis script to install plug-ins
  • update regression tutorial with commands to install plug-ins

This depends on PR #927

@@ -48,6 +48,10 @@ install:
- pip install -r requirements.txt
- pip install -r requirements_test.txt

# image gradients plug-ins
- pip install plugins/data/imageGradients
- pip install plugins/view/imageGradients
Copy link
Member

Choose a reason for hiding this comment

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

So... you don't actually have to pip install DIGITS? You only have to install the plugins?

Copy link
Member

Choose a reason for hiding this comment

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

No wait, yes you do.

    Traceback (most recent call last):
      File "<string>", line 1, in <module>
      File "/tmp/pip-59l1vv-build/setup.py", line 5, in <module>
        from digits.extensions.data import GROUP as DIGITS_PLUGIN_GROUP
    ImportError: No module named digits.extensions.data

I think this Travis build is about to fail, let's see ...
https://travis-ci.org/NVIDIA/DIGITS/builds/162241842

@lukeyeager
Copy link
Member

This was the only extension that was hidden by default, right? Can we just get rid of the DIGITS_ALL_EXTENSIONS envvar then?
https://github.com/NVIDIA/DIGITS/blob/f2b0b0a640/digits/config/extension_list.py

It seems to me that we can entirely replace the show/hide+envvar system with the uninstalled/installed system.

@gheinrich gheinrich force-pushed the dev/gradients-plugin branch 2 times, most recently from a8f6b1f to a297ca1 Compare September 26, 2016 06:53
@gheinrich
Copy link
Contributor Author

we can entirely replace the show/hide+envvar system with the uninstalled/installed system

Sure, this was my original intention but the change disappeared during the rebase. I have re-instated that change.

you don't actually have to pip install DIGITS?

You're right - I have updated the change, thanks.

(N) [none] <NONE>
>> A
Using "image-bounding-boxes,image-gradients,all-raw-data"
$ pip install -e $DIGITS_HOME
Copy link
Member

Choose a reason for hiding this comment

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

Do you mind if we use _ROOT instead of _HOME here? See 800e352.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not at all, updated in latest commit

import os
from setuptools import setup, find_packages

# Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
Copy link
Member

Choose a reason for hiding this comment

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

Any reason why the copyright isn't at the top of this file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch, thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated in latest commit

@lukeyeager lukeyeager merged commit 88666fa into NVIDIA:master Sep 26, 2016
@gheinrich gheinrich deleted the dev/gradients-plugin branch November 30, 2016 16:48
SlipknotTN pushed a commit to cynnyx/DIGITS that referenced this pull request Mar 30, 2017
Move image gradient extensions to plug-in area
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants