search

swcarpentry / DEPRECATED-bc

DEPRECATED: This repository is now frozen - please see individual lesson repositories.
Other
299 stars 383 forks source link

Translating the Python lesson on conditionals to MATLAB #709

Closed shwina closed 9 years ago

shwina commented 9 years ago

This is an early pull request to resolve #708. There are a few FIXMES, but I think an early review might help.

rgaiacs commented 9 years ago

@ashwinsrnth You are doing great with the translation. Let me know if you need anything,

shwina commented 9 years ago

As always, thanks for the helpful review, Raniere. I'll fix these things soon

shwina commented 9 years ago

@r-gaia-cs I've addressed the issues you mentioned. Let me know if this is good. Thanks!

rgaiacs commented 9 years ago

@ashwinsrnth Just a few minor changes.

DamienIrving commented 9 years ago

Hi @ashwinsrnth and @r-gaia-cs. Do you think it might be worth separating this PR out into two separate PRs? I'm thinking:

  1. A PR with all the minor changes to lessons 1, 2 and 3. This could be accepted/merged into the bc repo pretty soon (i.e. once @ashwinsrnth has finished making the minor changes he'd like to make) so that @IsaKiko can then fork the bc repo and add her minor changes to lessons 1, 2 and 3
  2. A second PR with the new lesson 4

@IsaKiko could then create a third PR with the new lesson 5 that she is working on.

How does that sound? I figure it's the easiest way to coordinate all the changes that are going on. Otherwise for @IsaKiko to review lessons 1, 2 and 3 she'd have to fork @ashwinsrnth's fork, which starts to get a little messy...

rgaiacs commented 9 years ago

Hi @DamienIrving. I had talk with @ashwinsrnth about this (use the same PR for minor and major changes) but didn't ask to split this PR. If this is blocking the work of other contributor I'm +1 to make this change.

@ashwinsrnth Could you do what @DamienIrving request? If you need help just call me.

shwina commented 9 years ago

@DamienIrving @r-gaia-cs @IsaKiKo

Everyone, sorry about the bulky PRs. I'll open the new PR (for 1, 2, and 3) and begin working on it. Maybe @IsaKiKo can begin working on lesson 5 in parallel? As for the content of lesson 5, can we put that under discussion?

For reasons that Greg Wilson mentioned in yesterday's lab meeting, the Python lessons don't teach how to use a unit-testing framework. They do teach how to use assertions to write more robust code. Should the MATLAB lessons follow suit, and not teach matlab.unittest? Maybe that belongs in the intermediate materials?

I realize this discussion doesn't belong in this PR, but I want to bring it up before @IsaKiKo begins investing time on lesson 5. Maybe we can continue this discussion on the mailing list.

Thanks!

DamienIrving commented 9 years ago

@ashwinsrnth I agree that we should follow exactly what the Python lessons do with respect to defensive programming - introduce assertions and the idea of test driven development and writing test functions, but we can leave matlab.unittest to the intermediate materials.

Since assertions in Matlab are pretty much exactly the same as they are in Python (see here) the translation from Python to Matlab should be pretty easy.

I think once @IsaKiko submits a pull request on lesson 5 we can continue this discussion on that PR if need be.

shwina commented 9 years ago

@DamienIrving

Sounds good to me, thanks!

gvwilson commented 9 years ago

Can't assign to @ashwinsrnth yet, so flatting this way.

shwina commented 9 years ago

@r-gaia-cs

I've fixed all the trailing whitespace issues. Can we merge this? I promise to make smaller PRs henceforth :)

rgaiacs commented 9 years ago

@ashwinsrnth Could you please fix my last three comments?

shwina commented 9 years ago

@r-gaia-cs

Done