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

08-putting-it-all-together.md: rewrite to use random data and draw histograms. #320

Merged
merged 24 commits into from Oct 19, 2018

Conversation

@wrightaprilm
Copy link
Contributor

@wrightaprilm wrightaprilm commented Oct 5, 2018

Hi team, I was teaching with the new Data Ingest lesson the other day, and had a little trouble motivating the students. I made a couple changes that might be helpful:

  1. Changed the multiaxis plot example from a line to a distribution. I did this because drawing from a statistical distribution to get a sense for its properties is a common operation - we're not statisticians, so we need some help ;)
  2. Changed the summative flood plotting exercise to a multiaxis plot to show how you could use multiaxis plots to call attention to something of interest.

I'd love feedback! Particularly from you, @stijnvanhoey; I know (and appreciate deeply) how much effort you put in to this lesson.

@wrightaprilm wrightaprilm requested a review from maxim-belkin Oct 5, 2018
@maxim-belkin
Copy link
Contributor

@maxim-belkin maxim-belkin commented Oct 5, 2018

This PR has some conflicts. I'd suggest you rebase it on top of the latest gh-pages branch:

git checkout -f gh-pages
git fetch https://github.com/datacarpentry/python-ecology-lesson
git reset --hard FETCH_HEAD
git rebase gh-pages distn
git add _episodes/08-putting-it-all-together.md
git rebase --continue

Loading

@wrightaprilm
Copy link
Contributor Author

@wrightaprilm wrightaprilm commented Oct 5, 2018

Did the rebase. Is rebase more verbose than it used to be? I used to find doing that so confusing.

Loading

@stijnvanhoey
Copy link
Contributor

@stijnvanhoey stijnvanhoey commented Oct 6, 2018

@wrightaprilm testing and trying out the material is the real way to improve and adapt the material, thanks! I'll have a next test-round in December this year...

With respect ot your PR, I do like your adaptations, so this would be fine by me (I'll have a look at the code itself and try to provide some useful suggestions). Although I would change the exercise of the summative flood plotting to adding an orange/red 'flood warning line' + text annotation alarm level instead of the multiaxis? Adding 'annotations' is maybe a more frequently encountered need?

At the same time, reading the material again with respect to your comment on motivation, the Putting it all together statement is maybe setting the expectations too high for the participants? The title data ingest and visualisation could be kept, but maybe we should start the lesson motivating them from an alternative perspective, e.g. the real world is messy (data ingest is not just read.csv as we have to deal with strange headers,...) and never trust the defaults (to create graphs for publication, specific adaptations are required, e.g. annotations, and we need to properly save them)?

Loading

Copy link
Contributor

@stijnvanhoey stijnvanhoey left a comment

See my suggestion about the exercise. Feel free to accept of reject it.

Loading

_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
@wrightaprilm wrightaprilm changed the title Distn Change challenge in 08 Oct 8, 2018
@wrightaprilm
Copy link
Contributor Author

@wrightaprilm wrightaprilm commented Oct 8, 2018

At the same time, reading the material again with respect to your comment on motivation, the Putting it all together statement is maybe setting the expectations too high for the participants? The title data ingest and visualisation could be kept, but maybe we should start the lesson motivating them from an alternative perspective, e.g. the real world is messy (data ingest is not just read.csv as we have to deal with strange headers,...) and never trust the defaults (to create graphs for publication, specific adaptations are required, e.g. annotations, and we need to properly save them)?

Good point - a little more explanation on why we're doing another plot library after just learning one might be in order.

Loading

@wrightaprilm
Copy link
Contributor Author

@wrightaprilm wrightaprilm commented Oct 15, 2018

I had raised the issue of putting a little more why another plotting lib. I think that's a good idea, but a bit past the scope of the PR as it stands. If we're happy, I think we merge this, and I open a help-wanted issue. Does that sound OK to y'all?

Loading

@stijnvanhoey
Copy link
Contributor

@stijnvanhoey stijnvanhoey commented Oct 15, 2018

Good idea, fine for me!

Loading

_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
> using matplotlib.
> (September 11 through 15) and create a hydrograph (line plot) of the discharge data using
> Pandas, linking it to an empty maptlotlib `ax` object. Create a second axis that is the
> whole dataset. Adapt the title, x-axis and y-axis label using matplotlib.
Copy link
Contributor

@maxim-belkin maxim-belkin Oct 15, 2018

Choose a reason for hiding this comment

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

  1. labels.
  2. I'd suggest something like:

Adapt the title and axes' labels using matplotlib

Loading

Copy link
Contributor

@maxim-belkin maxim-belkin left a comment

Please have a look at my comments. In general - looks great!

Loading

_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
_episodes/08-putting-it-all-together.md Outdated Show resolved Hide resolved
Loading
@wrightaprilm
Copy link
Contributor Author

@wrightaprilm wrightaprilm commented Oct 19, 2018

I think I got everything, but I'd like to change filenames and remove extraneous plot files in a separate PR.

Loading

@maxim-belkin maxim-belkin changed the title Change challenge in 08 08-putting-it-all-together.md: rewrite to use random data and draw histograms. Oct 19, 2018
@maxim-belkin maxim-belkin merged commit 4f44253 into datacarpentry:gh-pages Oct 19, 2018
@maxim-belkin
Copy link
Contributor

@maxim-belkin maxim-belkin commented Oct 19, 2018

Awesome work, @wrightaprilm!

Loading

@wrightaprilm
Copy link
Contributor Author

@wrightaprilm wrightaprilm commented Oct 19, 2018

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants