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

Little fix in function for recursive find sources #95

Merged
merged 2 commits into from
Jan 20, 2021

Conversation

yasakova-anastasia
Copy link

@yasakova-anastasia yasakova-anastasia commented Jan 19, 2021

Summary

  • Little fix in _find_sources_recursive

How to test

Checklist

License

  • I submit my code changes under the same MIT License that covers the project.
    Feel free to contact the maintainers if that's a concern.
  • I have updated the license header for each file (see an example below)
# Copyright (C) 2020 Intel Corporation
#
# SPDX-License-Identifier: MIT

@@ -624,10 +624,10 @@ def _find_sources_recursive(cls, path, ext, extractor_name,
for d in range(max_depth + 1):
sources.extend({'url': p, 'format': extractor_name} for p in
iglob(osp.join(path, *('*' * d), dirname, filename + ext)))
if callable(file_filter):
Copy link
Contributor

Choose a reason for hiding this comment

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

This way the function will be called on each iteration, and will re-check items from previous steps. We can move it out of the loop or include inside of extend(...).

Copy link
Contributor

@zhiltsov-max zhiltsov-max left a comment

Choose a reason for hiding this comment

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

What does this fix solve?

@yasakova-anastasia
Copy link
Author

What does this fix solve?

We can find the file at the initial iteration and exit the loop. Then it may not pass filtering and sources will be empty.

@zhiltsov-max zhiltsov-max merged commit 30c0648 into develop Jan 20, 2021
@zhiltsov-max zhiltsov-max deleted the ay/little-fixes branch January 20, 2021 09:07
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