-
Notifications
You must be signed in to change notification settings - Fork 43
pattern: FOR pattern correctness and unit test #40
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -257,8 +257,6 @@ def FOR(get_list_function, setter, branch, cache_data=False, order="ASC"): | |
| :param get_list_function: function returning the list on which we should | ||
| iterate. | ||
| :param branch: block of functions to run | ||
| :param savename: name of variable to save the current loop state in the | ||
| extra_data in case you want to reuse the value somewhere in a task. | ||
| :param cache_data: can be True or False in case of True, the list will be | ||
| cached in memory instead of being recomputed everytime. In case of caching | ||
| the list is no more dynamic. | ||
|
|
@@ -268,8 +266,6 @@ def FOR(get_list_function, setter, branch, cache_data=False, order="ASC"): | |
| :param setter: function to call in order to save the current item of the | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The usage that is done of this on line 267 is not documented here right? It seems as if you could pass a string as |
||
| list that is being iterated over. | ||
| expected to take arguments (obj, eng, val) | ||
| :param getter: function to call in order to retrieve the current item of | ||
| the list that is being iterated over. expected to take arguments(obj, eng) | ||
| """ | ||
| # be sane | ||
| assert order in ('ASC', 'DSC') | ||
|
|
@@ -292,49 +288,56 @@ def get_list(): | |
| return eng.extra_data["_Iterators"][step]["cache"] | ||
| except KeyError: | ||
| if callable(get_list_function): | ||
| return get_list() | ||
| return get_list_function() | ||
| elif isinstance(get_list_function, collections.Iterable): | ||
| return list(get_list_function) | ||
| else: | ||
| raise TypeError("get_list_function is not a callable nor a" | ||
| " iterable") | ||
| raise TypeError("get_list_function is not callable nor an" | ||
| " iterable.") | ||
|
|
||
| my_list_to_process = get_list() | ||
| list_to_process = get_list() | ||
|
|
||
| # First time we are in this step | ||
| if step not in eng.extra_data["_Iterators"]: | ||
| eng.extra_data["_Iterators"][step] = {} | ||
| # Cache list | ||
| if cache_data: | ||
| eng.extra_data["_Iterators"][step]["cache"] = get_list() | ||
| eng.extra_data["_Iterators"][step]["cache"] = list_to_process | ||
| # Initialize step value | ||
| eng.extra_data["_Iterators"][step]["value"] = { | ||
| "ASC": 0, | ||
| "DSC": len(my_list_to_process) - 1}[order] | ||
| "DSC": len(list_to_process) - 1}[order] | ||
| # Store previous data | ||
| if 'current_data' in eng.extra_data["_Iterators"][step]: | ||
| eng.extra_data["_Iterators"][step]["previous_data"] = \ | ||
| eng.extra_data["_Iterators"][step]["current_data"] | ||
|
|
||
| elif 'current_data' in eng.extra_data["_Iterators"][step]: | ||
| eng.extra_data["_Iterators"][step]["previous_data"] = \ | ||
| eng.extra_data["_Iterators"][step]["current_data"] | ||
|
|
||
| # Increment or decrement step value | ||
| step_value = eng.extra_data["_Iterators"][step]["value"] | ||
| currently_within_list_bounds = \ | ||
| (order == "ASC" and step_value < len(my_list_to_process)) or \ | ||
| (order == "ASC" and step_value < len(list_to_process)) or \ | ||
| (order == "DSC" and step_value > -1) | ||
| if currently_within_list_bounds: | ||
| # Store current data for ourselves | ||
| eng.extra_data["_Iterators"][step]["current_data"] = \ | ||
| my_list_to_process[step_value] | ||
| list_to_process[step_value] | ||
| # Store for the user | ||
| if setter: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When is
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I know that it's not part of your patch, just trying to understand it, I don't mean that you should change this in this pr, but will help me understand the code, and thus this pr, and hopefully help you 😉)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, if you have a question reading the code it means it is not clear enough and should be rework/commented, so no problem, it is a great addition. Best |
||
| setter(obj, eng, step, my_list_to_process[step_value]) | ||
| setter(obj, eng, step, list_to_process[step_value]) | ||
| if order == 'ASC': | ||
| eng.extra_data["_Iterators"][step]["value"] += 1 | ||
| elif order == 'DSC': | ||
| eng.extra_data["_Iterators"][step]["value"] -= 1 | ||
| else: | ||
| setter(obj, eng, step, | ||
| eng.extra_data["_Iterators"][step]["previous_data"]) | ||
| # Special case were no iteration is needed. | ||
| if "previous_data" in eng.extra_data["_Iterators"][step]: | ||
| setter(obj, eng, step, | ||
| eng.extra_data["_Iterators"][step]["previous_data"]) | ||
| else: | ||
| # We set None as no value should have been generated if | ||
| # no iteration has been done. | ||
| setter(obj, eng, step, None) | ||
| del eng.extra_data["_Iterators"][step] | ||
| eng.break_current_loop() | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🦄