EntilZha / PyFunctional

Python library for creating data pipelines with chain functional programming
http://pyfunctional.pedro.ai
MIT License
2.41k stars 132 forks source link

add remove_all in seq #188

Closed ponytailer closed 1 year ago

ponytailer commented 1 year ago

remove the members which in seq.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (a4cd31c) 98.08% compared to head (a0d165b) 98.09%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #188 +/- ## ======================================= Coverage 98.08% 98.09% ======================================= Files 12 12 Lines 2345 2357 +12 ======================================= + Hits 2300 2312 +12 Misses 45 45 ``` | [Impacted Files](https://app.codecov.io/gh/EntilZha/PyFunctional/pull/188?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pedro+Rodriguez) | Coverage Δ | | |---|---|---| | [functional/pipeline.py](https://app.codecov.io/gh/EntilZha/PyFunctional/pull/188?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pedro+Rodriguez#diff-ZnVuY3Rpb25hbC9waXBlbGluZS5weQ==) | `98.12% <100.00%> (+0.01%)` | :arrow_up: | | [functional/test/test\_functional.py](https://app.codecov.io/gh/EntilZha/PyFunctional/pull/188?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Pedro+Rodriguez#diff-ZnVuY3Rpb25hbC90ZXN0L3Rlc3RfZnVuY3Rpb25hbC5weQ==) | `99.03% <100.00%> (+<0.01%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

EntilZha commented 1 year ago

Thanks for the interest in adding new features!

As with other new API additions, is there a pre-existing API in python (eg, itertools or functools) or related (e.g., spark or scala) that this is based on? Similarly, could the use case be satisfied by using custom registering/extend (this really should be in README.md regardless...) https://github.com/EntilZha/PyFunctional/pull/144? As with other new API additions, I'm in general hesitant to add too much, but if there is a good case for it being (1) commonly used and/or (2) difficult to replicate otherwise, then I could be convinced (e.g., I am mostly convinced about https://github.com/EntilZha/PyFunctional/pull/183/files, just need to look over it again since it looks like there was some comments on the code by others).

Aside from that, the current PyFunctional APIs are pretty functional, going from the unit test examples, source.remove_all() looks like it happens in place, which doesn't conform to the rest of the API. As @FlavioAmurrioCS mentions, the library API is built around returning sequences that apply the changes. For example, something that would confirm to the current API style would be:

source = self.seq([1, 2, 3, 1]).map(lambda x: x)
new_source = source.remove_all([3, 100])
# new_source now doesn't have 3
ponytailer commented 1 year ago

Thanks for the interest in adding new features!

As with other new API additions, is there a pre-existing API in python (eg, itertools or functools) or related (e.g., spark or scala) that this is based on? Similarly, could the use case be satisfied by using custom registering/extend (this really should be in README.md regardless...) #144? As with other new API additions, I'm in general hesitant to add too much, but if there is a good case for it being (1) commonly used and/or (2) difficult to replicate otherwise, then I could be convinced (e.g., I am mostly convinced about https://github.com/EntilZha/PyFunctional/pull/183/files, just need to look over it again since it looks like there was some comments on the code by others).

Aside from that, the current PyFunctional APIs are pretty functional, going from the unit test examples, source.remove_all() looks like it happens in place, which doesn't conform to the rest of the API. As @FlavioAmurrioCS mentions, the library API is built around returning sequences that apply the changes. For example, something that would confirm to the current API style would be:

source = self.seq([1, 2, 3, 1]).map(lambda x: x)
new_source = source.remove_all([3, 100])
# new_source now doesn't have 3

OK, I understood.