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

cytoolz.curried.valmap suppresses errors in the mapping function call #135

Open
kevinheavey opened this issue Oct 2, 2019 · 2 comments
Open

Comments

@kevinheavey
Copy link

kevinheavey commented Oct 2, 2019

Here is how valmap works when it runs into an error inside the mapping function like below:

In [1]: import cytoolz

In [2]: cytoolz.curried.valmap(lambda x: x + 'a')({'key': 1})
Out[2]: <cyfunction valmap at 0x7f7791339b90>

Meanwhile here's how toolz works (raises an error as expected):

In [3]: import toolz

In [4]: toolz.curried.valmap(lambda x: x + 'a')({'key': 1})
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-9d36150f91b0> in <module>
----> 1 toolz.curried.valmap(lambda x: x + 'a')({'key': 1})

/tmp/envs/my_env/lib/python3.7/site-packages/toolz/functoolz.py in __call__(self, *args, **kwargs)
    301     def __call__(self, *args, **kwargs):
    302         try:
--> 303             return self._partial(*args, **kwargs)
    304         except TypeError as exc:
    305             if self._should_curry(args, kwargs, exc):

/tmp/envs/my_env/lib/python3.7/site-packages/toolz/dicttoolz.py in valmap(func, d, factory)
     82     rv = factory()
---> 83     rv.update(zip(iterkeys(d), map(func, itervalues(d))))
     84     return rv
     85

<ipython-input-4-9d36150f91b0> in <lambda>(x)
----> 1 toolz.curried.valmap(lambda x: x + 'a')({'key': 1})

TypeError: unsupported operand type(s) for +: 'int' and 'str'

Package versions:

In [5]: cytoolz.__version__
Out[5]: '0.10.0'

In [6]: toolz.__version__
Out[6]: '0.10.0'
@eriknw
Copy link
Member

eriknw commented Oct 11, 2019

Aha, thanks! curry._should_curry method returns True when it should return False when cytoolz.valmap is curried. This is true when using both toolz.curry and cytoolz.curry.

Apparently, Python now provides an inspect.Signature object for the Cython function cytoolz.valmap. It gives <Signature (func, d, factory)>, but it should give <Signature (func, d, factory=<class 'dict'>)>. It doesn't know that factory has a default value.

Is there a way for Cython to provide an accurate signature? If not, we can rely on the info in toolz._signatures to provide the correct behavior again.

@eriknw
Copy link
Member

eriknw commented Oct 11, 2019

It looks like this is indeed an issue in Cython (cython/cython#1864). We could apply a bandaid for functions in cytoolz by giving priority to the signatures defined here: https://github.com/pytoolz/cytoolz/blob/master/cytoolz/_signatures.py

I can go ahead and do this, which should fix the issue. Thanks again for the report and regression test @kevinheavey!

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

No branches or pull requests

2 participants