Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added ignore kwarg to traverse() in common, allowing directory pruning #209

Merged
merged 4 commits into from
Feb 28, 2021

Conversation

Cobertos
Copy link
Contributor

This adds an optional kwarg to traverse() in common.py, allowing directories that are ignored to be pruned by fdfind and find, instead of iterating over and excluding later.

There's also two tests to make sure that this is working with both fdfind and find. Passes on my machine

@Cobertos
Copy link
Contributor Author

Hmm, I forgot about the Windows implementation

@karlicoss
Copy link
Owner

karlicoss commented Feb 28, 2021 via email

@Cobertos
Copy link
Contributor Author

Oh okay, I already fixed x3

@Cobertos
Copy link
Contributor Author

Oh, I also forgot to mention that this requires more_itertools for the disperse function. I did not add the dependency, let me know the best way to handle that, considering all the different sort ways there are to install.

@karlicoss
Copy link
Owner

Ah, it's already a dependency:

'more_itertools',

so no problem with that. more_itertools is valuable enough to have as dependency (plus it's one of the most common dependencies anyway, so usually already installed)

@karlicoss
Copy link
Owner

Sorry, need to figure out how to run github CI against other's people's PRs -- at the moment it's only triggered by my own pushes only, so I need to checkout PR and push it to trigger.


Looks like it's failing ignore_find test (CI=true tox -e tests -- -k ignore_find)

E         Full diff:
E           [
E         +  PosixPath('/adhoc-project/tests/testdata/traverse/imhere.txt'),
E            PosixPath('/adhoc-project/tests/testdata/traverse/imhere2/real.txt'),
E         -  PosixPath('/adhoc-project/tests/testdata/traverse/imhere.txt'),
E           ]

guess find doesn't guarantee order.


mypy also is going to complain CI=true tox -e mypy-core:

src/promnesia/common.py: note: In function "find_args":
src/promnesia/common.py:401: error: Argument 1 to "chain" has incompatible type "*Iterator[List[str]]"; expected
"Iterable[List[str]]"  [arg-type]
            ignore_names = list(itertools.chain(*intersperse(['-o'], ignore_names)))
                                                 ^
src/promnesia/common.py:411: error: List item 2 has incompatible type "List[Sequence[str]]"; expected "str"  [list-item]
            *prune_dir_args,
             ^
src/promnesia/common.py:413: error: List item 5 has incompatible type "List[Sequence[str]]"; expected "str"  [list-item]
            *ignore_file_args
             ^
src/promnesia/common.py: note: In function "fdfind_args":
src/promnesia/common.py:425: error: Argument 1 to "chain" has incompatible type "*List[List[str]]"; expected
"Iterable[List[str]]"  [arg-type]
            ignore_args = list(itertools.chain(*ignore_args))
                                                ^
src/promnesia/common.py:429: error: List item 1 has incompatible type "List[List[str]]"; expected "str"  [list-item]
            *ignore_args,
             ^

If you allow edits from maintainers, I don't mind quickly fixing it.

@Cobertos
Copy link
Contributor Author

Cobertos commented Feb 28, 2021 via email

@karlicoss
Copy link
Owner

Ah, indeed, pushing onto the branch works, not sure why it didn't first time I tried!

Python typing stuff is mypy. In principle, possible to run it as mypy -p promnesia, but due to third party modules and other complexities, the most reliable is to do it via tox (e.g. CI=true tox -e mypy-core)

promnesia/tox.ini

Lines 32 to 39 in 708d073

[testenv:mypy-core]
commands =
pip install -e .[linting]
python -m mypy -p promnesia --exclude 'sources/*' \
# txt report is a bit more convenient to view on CI
--txt-report .coverage.mypy-core \
--html-report .coverage.mypy-core \
{posargs}

Either way, it was nothing major, just mypy being a bit picky about reusing the name with a different type, I quickly fixed it.

@Cobertos
Copy link
Contributor Author

Oh I see, I haven't used tox at all. I will have to look into how that's all set. Looks like you got the PR CI stuff working too.

@karlicoss karlicoss merged commit e44a2bd into karlicoss:master Feb 28, 2021
@karlicoss
Copy link
Owner

Thanks for the contribution!

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.

2 participants