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.

Sign up for GitHub

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

Jump to bottom

Fix IQR playground image build by upgrading pip, and also auto sense IMAGE_VERSION #379

Merged
merged 10 commits into from
Oct 31, 2018
Merged

Fix IQR playground image build by upgrading pip, and also auto sense IMAGE_VERSION #379

merged 10 commits into from
Oct 31, 2018

Conversation

chrismattmann
Copy link
Contributor

No description provided.

@chrismattmann
Copy link
Contributor Author

Fixes #376

Purg
Purg requested changes Oct 30, 2018
View reviewed changes
devops/docker/smqtk_caffe/Dockerfile.cpu.df Outdated Show resolved Hide resolved
devops/docker/smqtk_caffe/build_image.sh Outdated Show resolved Hide resolved
@chrismattmann
Copy link
Contributor Author

no problem @Purg updated per your comments.

@Purg
Copy link
Member

Purg commented Oct 30, 2018

Might want to also remove the "Build image without using cache" part of the PR title as that is not part of this PR anymore.

@chrismattmann chrismattmann changed the title Build image without using cache (for pip); and also pip the version of matplotlib to 2.2.2 b/c well sigh. Fix IQR playground image build by upgrading pip, and also auto sense IMAGE_VERSION Oct 30, 2018
@chrismattmann
Copy link
Contributor Author

got it, updated the title @Purg and will also go ahead and make the requested changes shortly.

@chrismattmann
Copy link
Contributor Author

all changes made

@Purg
Copy link
Member

Purg commented Oct 31, 2018

Changes LGTM. Building images to test.

@Purg
Copy link
Member

Purg commented Oct 31, 2018

We also need a rebase here.

@Purg
Copy link
Member

Purg commented Oct 31, 2018

Oops, apparently I can do that.

Purg
Purg approved these changes Oct 31, 2018
View reviewed changes
Copy link
Member

@Purg Purg left a comment

Choose a reason for hiding this comment

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

LGTM, Images built. Caffe importable and callable in python within both CPU and GPU image variants. Pip is correctly version 18.1 in both images as well.

Purg
Purg requested changes Oct 31, 2018
View reviewed changes
Copy link
Member

@Purg Purg left a comment •
edited

Choose a reason for hiding this comment

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

Almost forgot (sorry)! We need a change note. In docs/release_notes/pending_release.rst, add a bullet under the Fixed->Docker section for the Caffe image and that you resolved an issue with upgrading pip for a newer version of matplotlib.

@chrismattmann
Copy link
Contributor Author

chrismattmann commented Oct 31, 2018 •
edited

OK no problem release notes updated! @Purg

Purg
Purg approved these changes Oct 31, 2018
View reviewed changes
Copy link
Member

@Purg Purg left a comment

Choose a reason for hiding this comment

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

LGTM, waiting on successful CI.

@Purg Purg merged commit b844711 into Kitware:master Oct 31, 2018
@chrismattmann
Copy link
Contributor Author

awesome thanks @Purg

@chrismattmann chrismattmann mentioned this pull request Oct 31, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Purg Purg Purg approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@chrismattmann @Purg