Add refactor lessons as extra material #48
Conversation
|
I've finished the first 2 sections. I'll work on the Code Abstractions and MVC lessons tomorrow. |
| >> self.dir_path = dir_path | ||
| >> | ||
| >> def load_catchment_data(self): | ||
| >> data_file_paths = glob.glob(os.path.join(self.dir_path, 'rain_data_2015*.json')) |
There was a problem hiding this comment.
So I think these lessons are looking really good, my one niggle is with the arguments for how the files are being read - the controller still asks for 'Input CSV(s) containing measurement data', and is still only looking at the directory path and whether it ends with .json or .csv. I'm not sure how best to fix it, I think there should at least be some suggestion of how to fix this, or maybe some additional optional arguments in the next section (say -j puts it in json mode)?
There was a problem hiding this comment.
I agree that it's poor practice to look at the file endings for this work. We perhaps could include an exercise in the last lesson, for the students to replace this file ending check with a flag instead? I'll look at the options for this.
There was a problem hiding this comment.
But, for now, shall we merge this material? (and I'll add that exercise as another PR)
There was a problem hiding this comment.
Yeah, this is more of a comment than a requirement, happy for it to be merged as is
Adapting the code refactor lessons from carpentries-incubator#297
This material will be adapted for our use case. The new code is given: https://github.com/carpentries-incubator/python-intermediate-rivercatchment/tree/full-data-analysis