Skip to content

Implement walk() #3

Merged
thatch merged 4 commits intopython-packaging:mainfrom
spencerkee:implement-walk
Oct 29, 2020
Merged

Implement walk() #3
thatch merged 4 commits intopython-packaging:mainfrom
spencerkee:implement-walk

Conversation

@spencerkee
Copy link
Copy Markdown
Contributor

Closes #2

@@ -1,4 +1,5 @@
PYTHON?=python
SHELL := /bin/bash
Copy link
Copy Markdown
Contributor Author

@spencerkee spencerkee Oct 26, 2020

Choose a reason for hiding this comment

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

I set up a new environment, Windows Subsystem Linux for Ubuntu 20.04 LTS. I found it necessary to add this line because otherwise by default make uses /bin/sh as per https://www.gnu.org/software/make/manual/html_node/Choosing-the-Shell.html which causes make venv to fail with:

source .venv/bin/activate && make setup
/bin/sh: 1: source: not found
make: *** [Makefile:7: venv] Error 127

I'm not sure if you prefer users pass in the shell; if so I can remove this line, and then perhaps something to that nature should be added to the quick development instructions.

@@ -1,4 +1,5 @@
PYTHON?=python
SHELL := /bin/bash
PYTHON?=/usr/bin/env python3
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

At least in Ubuntu 20.04, by default python3 is installed to /usr/bin/python3 and /usr/bin/python is unset. Since this seems to be a python3 compatible project, I added this line.


.PHONY: setup
setup:
python -m pip install --upgrade wheel
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Similarly wheel is not included by default with pip on a fresh install of Ubuntu 20.04 LTS.

) -> Iterable[str]:
"""Generator function that calls os.walk on a path, applies includes/excludes, and yields the resulting paths

walk() uses fnmatch so Unix shell-style wildcards e.g. *, ?, [1-9] are supported. For more information on the wildcards see https://docs.python.org/3/library/fnmatch.html. Note, the patterns in includes/excludes are only applied to the filename itself, not the directories in the path.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I wonder why make lint didn't complain about this line length? I can reduce to 88.

@@ -0,0 +1,58 @@
import os
import tempfile
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not familiar with the use of volatile over tempfile for creating temporary directories, I can switch too volatile if you prefer.


DEFAULT_INCLUDES = ["*.py"]

DEFAULT_EXCLUDES = [
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm not exactly sure how black's default excludes works. On first glance it seems like it's a substring match which is why I added the asterisk to both sides of the string. I should also add a permalink to black here.

@spencerkee spencerkee mentioned this pull request Oct 26, 2020
@thatch
Copy link
Copy Markdown
Member

thatch commented Oct 29, 2020

Works for me, I'll make some minor changes before cutting the next release. Thanks again!

@thatch thatch merged commit 90286ac into python-packaging:main Oct 29, 2020
@spencerkee
Copy link
Copy Markdown
Contributor Author

Hi @thatch, any chance you can add a hacktoberfest-accepted label to this pr?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implementing walk

2 participants