datacarpentry / image-processing

Image Processing with Python
https://datacarpentry.org/image-processing
Other
99 stars 122 forks source link

Matplotlib is currently used in a hybrid way (explicit vs. implicit interfaces) #319

Closed mkcor closed 8 months ago

mkcor commented 11 months ago

Reference: https://matplotlib.org/stable/users/explain/quick_start.html#the-explicit-and-the-implicit-interfaces

I believe that we should use the "object-oriented (OO) style" throughout the lesson, since we have created figure and axis objects. At the moment, we are mixing up the OO-style with the pyplot-style. For instance, 328d544ab9079b5e0c851cc3bf470da8b126cce0 adds figure and axis objects, but we still have all the pyplot-style legacy. I would change, e.g.,

fig, ax = plt.subplots()
plt.plot(bin_edges[0:-1], histogram)
plt.title("Grayscale Histogram")
plt.xlabel("grayscale value")
plt.ylabel("pixels")
plt.xlim(0, 1.0)

into

fig, ax = plt.subplots()
ax.plot(bin_edges[0:-1], histogram)
ax.set_title("Grayscale Histogram")
ax.set_xlabel("grayscale value")
ax.set_ylabel("pixels")
ax.set_xlim(0, 1.0);

/cc @datacarpentry/image-processing-curriculum-maintainers

_Originally posted by @mkcor in https://github.com/datacarpentry/image-processing/pull/316#discussion_r1431508063_

quist00 commented 11 months ago

I would personally favor this change for several reasons, but I would like it to be something the curriculum advisory committee encourage for the carpentries as a whole rather than just this lesson. As @JeremyPike surmised, that line was originally added as a hack to eliminate the need to scroll to the top of the jupyter file every time you displayed a new image. Perhaps there is another way to accomplish that without using an oo style line, and retain the r-style if that is what the CAC ultimately favored.

I think during the move to beta we assumed this was like a 201 level carpentries workshop building on what they learned previously, so retaining the ggplot style they likely learned from more introductory carpentries workshops would be a good thing as it eliminates some cognitive load as they grapple with many other new concepts. New learners might also feel like they are just being asked to unlearn what they just learned.

I assume the introductory lessons adopt the ggplot approach because it is seen as slightly less complicated, especially for a subset that used ggplot with R, but I think front loading them with a little bit more will be worth it in the long run because I always encourage workshop attendees to use https://matplotlib.org/stable/gallery/ as a means to jumpstart their own plotting, and it relies almost exclusively on oo method when it is not using SNS or some other wrapper library.

uschille commented 9 months ago

I believe that we should use the "object-oriented (OO) style" throughout the lesson, since we have created figure and axis objects. At the moment, we are mixing up the OO-style with the pyplot-style. For instance, https://github.com/datacarpentry/image-processing/commit/328d544ab9079b5e0c851cc3bf470da8b126cce0 adds figure and axis objects, but we still have all the pyplot-style legacy. I would change, e.g.,

I personally would also favor @mkcor's suggestion. I think the current mix-up is due to the history of the lesson. Originally, the lesson used pyplot-style but when we transitioned to using Jupyter notebooks with %matplotlib widget it became necessary to create fig and ax objects for each cell. On that note, when I recently taught the lesson, I realized that %matplotlib widget does not suit certain platforms including Google Colab and Spyder. I intend to investigate alternatives when I find time.

tobyhodges commented 9 months ago

It seems we have (lazy) consensus and I would welcome a PR to address this across all episodes, whether from @mkcor or anyone else reading this who is willing to take the time.

@uschille please let us know what you discover regarding %matplotlib widget. Perhaps a good discussion point at a Maintainer call soon?

mkcor commented 8 months ago

@GParolini could help out with this issue (see https://github.com/scikit-image/scikit-image/issues/7330#issuecomment-1971180527). Many thanks to her!