Peer Feedback for milestone 4

[katieb1]

Here are the actionable items for milestone 4 based on the peer feedback we received (UBC-MDS/software-review#15)

• Note: I removed duplicate feedback from reviewers

Reviewer 1:

• Documentation:

• Remove k argument from tunnel_filter()

• Fix any typos in examples (we should run the examples on our own computers to make sure they work)

• Add example for colour_filters()

• Add inline comments to function code

• Either remove random images from root directory or move them to an images folder

• Functionality:

• Fix importation code (should be from pymagine import vignette_filter instead of from pymagine import pymagine etc.)

Reviewer 2:

• Documentation:

• Function names should be consistent (e.g. vignette_filter vs. colour_filters)

• Functionality:

• There is apparently a bug in the edge_detection filter where it is returning black images -> investigate this and see if it can be fixed

• Remove shape values output from tunnel_filter

• Index file name of saved images so that they don't overwrite each other (this will be useful if the user wants to input a list or array of images)

• Print image directly with imshow() (I know we already tried this, but it might be worth revisiting if we should have an option to do this?)

• Add a separate function argument for saving image to a folder

Now we should have a discussion of which tasks are necessary/plausible to include, and who will do each task.

[katieb1]

The issues I can tackle:

• Remove k argument from tunnel_filter
• Remove images from root directory
• Remove shape value output from tunnel_filter
• Add function argument for saving image to a folder/naming the output image (this also fixes the indexing issue, since the user can specify the names of the images in a loop)

[Sukriti1312]

I can add example to colour_filters function

[brendoncampbell]

I will:

• fix the issue in my function example
• consider a couple more code comments, but I have a decent number already

Questions/comments:

• Agree function name consistency could be improved but not sure if its worth the hassle/risk at this point. Colour_filterS isn't the end of the world in my opinion because it actually contains a variety of effects, but for the edge detection I agree we could have included 'filter' somehow for consistency. Given the circumstances, I don't think its a huge deal and am fine to leave it
• I'm not clear on the 'from pymagine import vignette_filter' item. As far as I can see this is the way it's already done? Am I missing something?
• Don't really care to figure out the imshow() stuff at this stage given the issues we had, but agree that would be a nice improvement in the future

[katieb1]

Sounds good!

I agree with the function name consistency - I feel like at this point it is more of a "cosmetic" thing and I think the other feedback is more useful to focus on right now.

As for the from pymagine import vignette_filter, I fixed that in my latest PR. The issue was that in our initial example, the import instructions were from pymagine import pymagine and it needed to be from pymagine import vignette_filter AND THEN when calling the function we needed to write vignette_filter.vignette_filter() rather than just calling vignette_filter() directly.

[trevorkwan]

I have fixed these issues:

"There is apparently a bug in the edge_detection filter where it is returning black images -> investigate this and see if it can be fixed"

"Add inline comments to function code"