Skip to content

Conversation

@rpalo
Copy link
Contributor

@rpalo rpalo commented Oct 21, 2020

Where to put new files:

  • New files should go into a top-level subfolder, named after the article slug. For example: my-awesome-article

How to merge your changes:

  1. Make sure the CI code style tests all pass (+ run the automatic code formatter if necessary).
  2. Find an RP Team member on Slack and ask them to review & approve your PR.
  3. Once the PR has one positive ("approved") review, GitHub lets you merge the PR.
  4. 🎉

@rpalo
Copy link
Contributor Author

rpalo commented Oct 21, 2020

Hm. Following the steps exactly in the CircleCI build/install/lint process on my machine gives no issues for Black.

Successful linting run.

Not sure how to proceed.

@dbader
Copy link
Member

dbader commented Oct 21, 2020

Hmm, any chance you're on a different version of black locally than what we use on the repo?

Here's the requirements file we're using on CI:

https://github.com/realpython/materials/blob/master/requirements.txt#L1-L2

@bzaczynski
Copy link
Contributor

@rpalo Yeah, it's a version issue. When I used the latest black, it didn't report any problems with those source files. However, after installing the one from the materials' requirements, it did complain about one file.

In this case, I'd argue that it makes sense to keep Ryan's original formatting. @dbader Is there a way to suppress a rule in black in an unobtrusive way? Alternatively, can we update the black version here?

$ black --version
black, version 20.8b1
$ black . --diff
All done! ✨ 🍰 ✨
5 files would be left unchanged.
$ black --version
black, version 19.10b0
$ black . --diff
--- durer.py    2020-10-22 10:34:18.389230 +0000
+++ durer.py    2020-10-22 10:39:17.843831 +0000
@@ -1,16 +1,11 @@
 """Validating Dürer's magic square."""
 
 import numpy as np
 
 square = np.array(
-    [
-        [16, 3, 2, 13],
-        [5, 10, 11, 8],
-        [9, 6, 7, 12],
-        [4, 15, 14, 1],
-    ]
+    [[16, 3, 2, 13], [5, 10, 11, 8], [9, 6, 7, 12], [4, 15, 14, 1],]
 )
 
 for i in range(4):
     assert square[i, :].sum() == 34
     assert square[:, i].sum() == 34
reformatted durer.py
All done! ✨ 🍰 ✨
1 file reformatted, 4 files left unchanged.

Comment on lines 1 to 3
# Code Materials for the Numpy Tutorial

These are code resources for the Numpy Tutorial article by @rpalo. No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

🔗 Link

Can you please include a link to the original article? Ideally, the link should spell the title:

Suggested change
# Code Materials for the Numpy Tutorial
These are code resources for the Numpy Tutorial article by @rpalo.
# NumPy Tutorial: First Steps in Data Science
This folder contains the sample code for the [NumPy Tutorial](https://realpython.com/numpy-tutorial/) by @rpalo.

Comment on lines 1 to 3
# Code Materials for the Numpy Tutorial

These are code resources for the Numpy Tutorial article by @rpalo. No newline at end of file
Copy link
Contributor

Choose a reason for hiding this comment

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

🤝 Handholding

I would also recommend showing the reader how to use those examples. For instance, show a command that installs NumPy and Matplotlib, which then could be quickly copied-and-pasted in the terminal:

$ python -m pip install numpy matplotlib

Better yet, add a requirements.txt file with all the dependencies with tested versions, and show how to use it:

$ python -m pip install -r requirements.txt

Show how to run the individual scripts and their outputs:

$ python normal.py
[-0.34057664  0.03159261 -0.46815197 -0.30809633 -0.37834691]
9534
10000
0.9534

import numpy as np
import matplotlib.image as mpimg

img = mpimg.imread("kitten.png")
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Wrong Filename

Suggested change
img = mpimg.imread("kitten.png")
img = mpimg.imread("kitty.jpg")

Comment on lines 4 to 5


Copy link
Contributor

Choose a reason for hiding this comment

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

🎩 Style Tip

Use consistent code formatting. Some of your files follow PEP 8 with respect to the number of blank lines, while others don't, for example.

print(type(img))
print(img.shape)

# => numpy.ndarray
Copy link
Contributor

Choose a reason for hiding this comment

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

👩‍💻 Technical Recommendation

Since you're showing output in most of your Python source files, maybe it would make sense to replace them with Jupyter Notebooks (.ipynb files)? They allow to save the expected output with them. What do you think?

(You can take a look at build-a-web-scraper/ folder, which comes with a few of those notebooks.)

@dbader
Copy link
Member

dbader commented Oct 26, 2020

FYI I bumped flake8 and black on master and that resolved the linter complaints on this PR :)

@rpalo
Copy link
Contributor Author

rpalo commented Oct 26, 2020

Woohoo! Thanks. I'm planning on resolving all of the other requests (along with the requests on the article draft) today or tomorrow.

Thanks again 😄

rpalo and others added 2 commits October 29, 2020 11:05
Co-authored-by: Bartosz Zaczyński <bartosz.zaczynski@gmail.com>
@bzaczynski bzaczynski self-requested a review October 30, 2020 13:21
Copy link
Contributor

@bzaczynski bzaczynski left a comment

Choose a reason for hiding this comment

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

@rpalo I just noticed a typo in the Maclauren name. It's actually Maclaurin with an "i" after Colin Maclaurin. Please update the relevant notebook as well as the file name. Thanks!

@bzaczynski
Copy link
Contributor

lgtm

@rpalo
Copy link
Contributor Author

rpalo commented Nov 27, 2020

Note, no changes to the code or anywhere else in the repo. Just re-running the image_mod.ipynb with higher res kitty images to match the images I'm using in the article to appease the preflight.

@jaschmitt jaschmitt merged commit 2da0a84 into realpython:master Dec 10, 2020
MiscStuff839 pushed a commit to MiscStuff839/materials that referenced this pull request Nov 16, 2025
Add materials for Numpy Tutorial article.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants