Skip to content
This repository has been archived by the owner on May 31, 2020. It is now read-only.

Remove ggplot #12

Merged
merged 5 commits into from
Mar 4, 2019
Merged

Remove ggplot #12

merged 5 commits into from
Mar 4, 2019

Conversation

ruebot
Copy link
Member

@ruebot ruebot commented Mar 2, 2019

DO NOT MERGE UNTIL AFTER #11 IS MERGED

Tested on default dataset and mounted data, all cells worked.

ruebot added 2 commits March 2, 2019 09:58
@ruebot
Setup Docker for notebooks
a8626fb 
- Add Dockerfile,
- Add required nltk data
- Add instructions in README
@ruebot
Resolve #9
90aa955 
- Rename notebook files
  - Create example notebook with existing data
  - Create notebook with empty collection id, and help text
- Update language in README and notebooks
- Tweak Dockerfile
@ruebot ruebot requested review from greebie and ianmilligan1 March 2, 2019 23:29
@ruebot ruebot mentioned this pull request Mar 2, 2019
Closed
@ruebot
Copy link
Member Author

ruebot commented Mar 3, 2019 •
edited
Loading

I tweaked the README, and made sure the auk-example-notebook was fully executed. Added it to this PR since the other two open ones are previous to this one; don't want to end up with merge conflicts 😱

@ruebot
Copy link
Member Author

ruebot commented Mar 3, 2019 •
edited
Loading

Even more fun, it all works with Binder when we add the Dockerfile 🎉

(tweak the User Configuration settings, and run all again)

@ruebot
Copy link
Member Author

ruebot commented Mar 3, 2019

...once everything is merged, we can add the badge to the example notebook.

Binder

@ruebot ruebot mentioned this pull request Mar 3, 2019
Merged
@greebie
Copy link
Collaborator

greebie commented Mar 3, 2019

lgtm once conflicts are resolved.

@ruebot
Copy link
Member Author

ruebot commented Mar 3, 2019

@greebie please wait until @ianmilligan1 reviews and tests as well. I'v configured this repo to have at least two reviewers review before a PR is merged now.

@ruebot
Copy link
Member Author

ruebot commented Mar 3, 2019

Commit message, when time to merge should read:

Title: Remove ggplot, and have auk-notebook-example fully executed. (#12)
Body:

- Remove ggplot
- Set auk-notebook-example to be fully executed
- Additional README tweaks

screenshot from 2019-03-03 16-17-06

ianmilligan1
ianmilligan1 approved these changes Mar 3, 2019
View reviewed changes
Copy link
Member

@ianmilligan1 ianmilligan1 left a comment

Choose a reason for hiding this comment

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

Looks good to me - I've tested, ggplot warnings are gone, and all cells ran and executed properly. Great stuff.

@greebie
Copy link
Collaborator

greebie commented Mar 4, 2019

Works good for me too.

@ianmilligan1 ianmilligan1 merged commit c1b1f7c into master Mar 4, 2019
@ianmilligan1 ianmilligan1 deleted the ggplot branch March 4, 2019 17:44
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@greebie greebie greebie approved these changes

@ianmilligan1 ianmilligan1 ianmilligan1 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.
3 participants
@ruebot @greebie @ianmilligan1