Issue 1114 #1131

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

Jump to bottom

Open

CucumisSativus wants to merge 10 commits into dry-python:master

from CucumisSativus:issue-1114

Open

Issue 1114 #1131

CucumisSativus wants to merge 10 commits into dry-python:master from CucumisSativus:issue-1114

Conversation

@CucumisSativus

Copy link

@CucumisSativus CucumisSativus commented Nov 16, 2021

I have made things!

Checklist

I have double checked that there are no unrelated changes in this pull request (old patches, accidental config files, etc)
I have created at least one test case for the changes I have made
I have updated the documentation for the changes I have made
I have added my changes to the CHANGELOG.md

Related issues

Refs #1114

A simple implementation of filter for maybe. I am happy to implement something similar for Result, but I'm not entirely sure about the preferred approach.
In Scala it looks more or less like this

 if (p(value)) this else Failure(new NoSuchElementException("Predicate does not hold for " + value))

Not sure if something like this is ok for Python as well.

My second idea was to have a signature like

def filter(predicate, on_failure):
(...)

so user can specify exactly what happens in case of not matching predicate
🙏 Please, if you or your company finds dry-python valuable, help us sustain the project by sponsoring it transparently on https://github.com/sponsors/dry-python. As a thank you, your profile/company logo will be added to our main README which receives hundreds of unique visitors per day.

@sobolevn

Copy link

Member

sobolevn commented Nov 16, 2021

We also need to make an interface type for filter. This way we will be able to use with HKT.
See interfaces/ folder.

@CucumisSativus

Copy link

Author

CucumisSativus commented Nov 18, 2021

@sobolevn I think i managed to add them, but lets say i'm not 100% sure about those. Looking forward for some feedback :)

sobolevn

sobolevn reviewed

Nov 18, 2021

View reviewed changes

Copy link

Member

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Just one question 🙂

returns/interfaces/filterable.py

"""

@abstractmethod

def filter(

Copy link

Member

@sobolevn sobolevn Nov 18, 2021

Choose a reason for hiding this comment

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

Future note: we can also make an overload for TypeGuard case.
See python/typeshed#6140

Copy link

Author

@CucumisSativus CucumisSativus Nov 20, 2021

Choose a reason for hiding this comment

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

Could you elaborate a bit what can we do with type guard here? We are not filtering for example a list with different types. Filter executed on _FilterableType always returns _FilterableType. Did I miss something?

Copy link

Member

@sobolevn sobolevn Nov 20, 2021

Choose a reason for hiding this comment

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

Nevermind, this was a self note 🙂
This is even not supported by mypy yet.

Copy link

Contributor

@ariebovenberg ariebovenberg Sep 12, 2022

Choose a reason for hiding this comment

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

It looks like mypy supports TypeGuard now 🎉 :

returns/maybe.py Outdated

"""Raises exception for successful container."""

raise UnwrapFailedError(self)

def filter(self, function):

Copy link

Member

@sobolevn sobolevn Nov 18, 2021

Choose a reason for hiding this comment

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

Is it possible to create a common filter_ function for all possible types? Result, IOResult, ...?

Copy link

Contributor

@ariebovenberg ariebovenberg Nov 18, 2021

Choose a reason for hiding this comment

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

filter is special in the sense that it 'empties' the container, which is possible in Maybe, but not in Result (i.e. what would the failure value of Success(4).filter(lambda x: False) be?)

Copy link

Member

@sobolevn sobolevn Nov 18, 2021

Choose a reason for hiding this comment

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

Ok, I see! Than, it looks like we can use MaybeLikeN to limit the input type of filter_?

Copy link

Author

@CucumisSativus CucumisSativus Nov 20, 2021

Choose a reason for hiding this comment

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

@sobolevn I used it, please tell me if its something you were looking for. I am not entirely sure if I did it right :)

@CucumisSativus

Copy link

Author

CucumisSativus commented Nov 23, 2021

@sobolevn when i'm running poetry run pytest typesafety -p no:cov -o addopts="" --mypy-ini-file=setup.cfg locally I get 830 passed, 1 warning. Any ideas why does it fail on github?

@sobolevn

Copy link

Member

sobolevn commented Nov 23, 2021

I will take a look a bit later this week.

Thanks a lot for the PR!

ariebovenberg

ariebovenberg reviewed

Nov 28, 2021

View reviewed changes

returns/interfaces/filterable.py Outdated

self: _FilterableType,

predicate: Callable[[_FirstType], bool],

) -> Kind1[_FilterableType, _FirstType]:

"""Applies 'predicate' to the result fo a previous computation."""

Copy link

Contributor

@ariebovenberg ariebovenberg Nov 28, 2021

Choose a reason for hiding this comment

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

typo: fo->of

ariebovenberg

ariebovenberg reviewed

Nov 28, 2021

View reviewed changes

returns/interfaces/filterable.py Outdated

>>> from returns.maybe import Nothing, Some

>>> from returns.pointfree import filter_

>>> def example(argument: int) -> bool:

Copy link

Contributor

@ariebovenberg ariebovenberg Nov 28, 2021

Choose a reason for hiding this comment

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

what would you think of naming the example function? Naming the function is_even makes the example clearer in my mind

@CucumisSativus CucumisSativus force-pushed the issue-1114 branch from f4bf1be to aa3a303 Compare

November 28, 2021 14:10

CucumisSativus added 9 commits

December 11, 2021 22:18

@CucumisSativus


 Filter for maybe

6ef0537

@CucumisSativus


 add changes to changelog

586af8b

@CucumisSativus


 Update docs

6b96d21

@CucumisSativus


 Update docs2

e281d41

@CucumisSativus


 hkt for filter

bed806e

@CucumisSativus


 Bind filter to maybe n

4954a4a

@CucumisSativus


 satisfy almighty mypy

45b1fa6

@CucumisSativus


 typo and nested function typing

@CucumisSativus


 issue found!

60950f6

@CucumisSativus CucumisSativus force-pushed the issue-1114 branch from e216c9c to 60950f6 Compare

December 11, 2021 21:19

@CucumisSativus

Copy link

Author

CucumisSativus commented Dec 11, 2021

@sobolevn i think I found it

@CucumisSativus


 slots added

cca6894

@codecov

Copy link

codecov bot commented Dec 12, 2021 •

edited

Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (b782c2e) to head (cca6894).
Report is 624 commits behind head on master.

Additional details and impacted files

@@ Coverage Diff @@
## master #1131 +/- ##
=========================================
 Coverage 100.00% 100.00% 
=========================================
 Files 79 81 +2 
 Lines 2415 2450 +35 
 Branches 208 210 +2 
=========================================
+ Hits 2415 2450 +35

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

donbarbos

donbarbos approved these changes

Jun 28, 2023

View reviewed changes

Labels

None yet

4 participants

@CucumisSativus @sobolevn @ariebovenberg @donbarbos

Uh oh!

Issue 1114 #1131

Are you sure you want to change the base?

Issue 1114 #1131

Uh oh!

Conversation

@CucumisSativus CucumisSativus commented Nov 16, 2021

I have made things!

Checklist

Related issues

Uh oh!

sobolevn commented Nov 16, 2021

Uh oh!

CucumisSativus commented Nov 18, 2021

Uh oh!

@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

@sobolevn sobolevn Nov 18, 2021

Choose a reason for hiding this comment

Uh oh!

@CucumisSativus CucumisSativus Nov 20, 2021

Choose a reason for hiding this comment

Uh oh!

@sobolevn sobolevn Nov 20, 2021

Choose a reason for hiding this comment

Uh oh!

@ariebovenberg ariebovenberg Sep 12, 2022

Choose a reason for hiding this comment

Uh oh!

@sobolevn sobolevn Nov 18, 2021

Choose a reason for hiding this comment

Uh oh!

@ariebovenberg ariebovenberg Nov 18, 2021

Choose a reason for hiding this comment

Uh oh!

@sobolevn sobolevn Nov 18, 2021

Choose a reason for hiding this comment

Uh oh!

@CucumisSativus CucumisSativus Nov 20, 2021

Choose a reason for hiding this comment

Uh oh!

CucumisSativus commented Nov 23, 2021

Uh oh!

sobolevn commented Nov 23, 2021

Uh oh!

@ariebovenberg ariebovenberg Nov 28, 2021

Choose a reason for hiding this comment

Uh oh!

@ariebovenberg ariebovenberg Nov 28, 2021

Choose a reason for hiding this comment

Uh oh!

CucumisSativus commented Dec 11, 2021

Uh oh!

codecov bot commented Dec 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

codecov bot commented Dec 12, 2021 •

edited

Loading