Hello,

The latest version of the pyrsistent dropped Python 2 support after one of the contributors added python_requires='>=3.5' in setup.py.

pip install pyrsistent Collecting pyrsistent Downloading https://files.pythonhosted.org/packages/83/14/6d02fad9caeb3903f06f9442e57789ca2fbb3cf7daf66d4de3aa4dc867dc/pyrsistent-0.17.1.tar.gz (106kB) 100% |████████████████████████████████| 112kB 604kB/s pyrsistent requires Python '>=3.5' but the running Python is 2.7.17

The fix added to declare Python 2 support drop seems to be ok but I think there is a problem with the sdist. I mean, what version of setuptools did you used to create the sdist? Because supporting python_requires requires setuptools>=24.2.0 and pip>=9.0.0 to benefit from it

https://packaging.python.org/guides/distributing-packages-using-setuptools/#python-requires

Wheels (.whl) for this package are currently missing from PyPI. Could wheels be uploaded for the current and future releases?

Read more about the advantages of wheels to understand why generating wheel distributions are important.

To create a wheel along with source distribution:

(venv) $ pip install --upgrade pip setuptools wheel
(venv) $ python setup.py sdist bdist_wheel

# See dist/*.whl

To upload wheels:

(venv) $ pip install twine
(venv) $ twine upload dist/*

This adds non-meaningful __getitem__s to the collection types so type hints involving the collection types do not raise an exception when using pytest. (Python itself ignores the type hints.)

This fixes the following issue when using Pyrsistent with type annotations and pytest: (names changed to protect the innocent)

tests/test.py:1: in <module>
    from a.b import C
<frozen importlib._bootstrap>:961: in _find_and_load
    ???
<frozen importlib._bootstrap>:950: in _find_and_load_unlocked
    ???
<frozen importlib._bootstrap>:646: in _load_unlocked
    ???
<frozen importlib._bootstrap>:616: in _load_backward_compatible
    ???
.tox/py36/lib/python3.6/site-packages/_pytest/assertion/rewrite.py:216: in load_module
    py.builtin.exec_(co, mod.__dict__)
a/b.py:203: in C
    def stuff(self) -> PVector[str]:
E   TypeError: 'ABCMeta' object is not subscriptable

>>> from pyrsistent import PRecord, field
>>> class A(PRecord):
...     x = field()
... 
>>> class B(PRecord):
...     x = field()
... 
>>> A(x=1) == B(x=1)
True

This is undesirable because it is idiomatic for different Python classes to represent different types. If objects of different type accidentally share the same field names and values, they should still be considered different.

By way of analogy,

>>> (1,) == [1]
False
>>> (1,) == {1}
False
>>> (1,) == "\x01"
False
>>> 

The single-letter constructors can be pretty annoying

• they're hard to search for in a module
• they very easily conflict with local variable names

It'd be nice if we had alternative, longer names for v, s, m, and b.

My branch adding a bunch of pyrsistent 0.9 usage is failing consistently on our build machines - but not on my local desktop. Sample failure:

twisted.trial.unittest.FailTest: not equal:
a = [frozenset([<Application(name=u'mysql-clusterhq', image=<object object at 0x7ffacf447ba0>, ports=frozenset([]), volume=None, links=frozenset([]), environment=None, memory_limit=None, cpu_shares=None, restart_policy=<RestartNever()>)>,
            <Application(name=u'site-clusterhq.com', image=<object object at 0x7ffacf447bb0>, ports=frozenset([]), volume=None, links=frozenset([]), environment=None, memory_limit=None, cpu_shares=None, restart_policy=<RestartNever()>)>]),
 u'example.com']
b = [pset([<Application(name=u'site-clusterhq.com', image=<object object at 0x7ffacf447bb0>, ports=frozenset([]), volume=None, links=frozenset([]), environment=None, memory_limit=None, cpu_shares=None, restart_policy=<RestartNever()>)>, <Application(name=u'mysql-clusterhq', image=<object object at 0x7ffacf447ba0>, ports=frozenset([]), volume=None, links=frozenset([]), environment=None, memory_limit=None, cpu_shares=None, restart_policy=<RestartNever()>)>]),
 u'example.com']

Those two seem like they ought to be equal, and on my computer they are. And yet. (Application is a class using the characteristic library; eventually we'll probably switch everything to pyrsistent).

Full set of failures: http://build.clusterhq.com/builders/flocker-ubuntu-14.04/builds/1272/steps/trial/logs/problems

We started noticing segfaults after introducing some hypothesis-based testing of some of our pyrsistent-using code. Here's a specific failure we ran into, https://clusterhq.atlassian.net/browse/FLOC-2913

I haven't constructed a minimal reproducing example yet (as I understand it, hypothesis is generating some random data and initializing a bunch of pyrsistent structures with it and some random data is causing the problem ... but I can't see which random data).

Maybe some hypothesis-based tests in pyrsistent would be a good way to narrow down the problem.

This is a bit of a wishlist feature.

I'd like to be able to define a data structure that looks like this:

class Leaf(PClass):
  foo = field(type=unicode)

class Tree(PClass):
  children = pvector_field((Tree, Leaf))

However, Python will object and say that Tree is not defined. Is there a way to specify the field's type after the class construction?

I'm writing code that pre-checked types looks like this:

class MyRec(PRecord):
    paths = field(type=PMap, initial=pmap(), factory=pmap, mandatory=True)

Switching to CheckedPMap, it's now something like this:

class MyRec(PRecord):
    class _PathMap(CheckedPMap):
        __key_type__ = unicode
        __value_type__ = FilePath
    paths = field(type=_PathMap, initial=_PathMap(), factory=_PathMap, mandatory=True)

This is rather verbose, but likely to be a common use case. I'm thinking that perhaps some utility functions would be helpful. For example:

class MyRec(PRecord):
    # If mandatory=True then automatically set initial too? Probably.
    paths = pmap_field(unicode, FilePath, mandatory=True)  # optional invariant keyword argument

If I end up writing this for work I'll submit a PR.

Fixes issue #81 in theory. In practice we might want to also update thaw:

# currently
thaw({'a': m(b=1)})
# returns {'a': m(b=1))}

# improved thaw
thaw({'a': m(b=1)})
# would return {'a': {'b': 1}}

If you think that makes sense I should prob make that change in this PR so we can add all the new functionality at once.

One other thing - I'm not a fan of the name nonstrict that I chose for the flag to make freeze ignore pmaps and pvectors. If you've got a better suggestion I'm very happy to change it.

PClass subclasses must be given keyword arguments for initialization, currently. This is often quite a good thing. However, particularly for a type with only one field, the ability to use a positional argument instead is rather convenient.

Compare:

self.assertThat(foo, MappingLikeEquals(expected=bar))`

vs

self.assertThat(foo, MappingLikeEquals(bar))`

It's hardly an earth shattering difference but I don't find the keyword argument adds anything to the former (and it stands out against the idiom of the API it's being used with, which includes other such types as Equals(bar), Contains(bar), etc).

Hi, I just started using pyrsistent, but it's working great so far. Thanks for maintaining it.

Here's some code that I dislike.

@dataclass_json
@dataclass(frozen=True)
class Foo:
    bar: Dict[str, str]

I dislike it because you might think that frozen=True means that you're in immutable-land, but it's actually quite possible to mutate objects of this type:

x = Foo({"a":"a"})
x.bar["b"] = "b"
print(x.to_json()) # {"bar": {"a": "a", "b": "b"}}

You can be disciplined, and have both an immutable dataclass and immutable field types:

@dataclass_json
@dataclass(frozen=True)
class Foo:
    bar: PMap[str, str]

But it's easy to forget/miscommunicate and accidentally add a mutable field. That's not pyrsistent's problem, but maybe pyrsistent is in a good position to provide a solution:

@dataclass_json
@dataclass_pyrsistent
@dataclass(frozen=True)
class Foo:
    bar: Dict[str, str]

The decorator would translate the mutable container type to PMap (or PVector) and call freeze on Dicts (or Lists) upon initialization. If you provide a type that pyrsistent can't translate (like a non-pyrsistent-dataclass) then it could just fail with a nice message.

I've included dataclass_json in this request as an example of a library that:

• does something similar
• might appear in context with the proposed decorator

Thanks for considering it!

Hi,

I was wondering if you would like to integrate continuous fuzzing by way of OSS-Fuzz? Fuzzing is a way to automate test-case generation and can be used to find unexpected exceptions in Python. In this PR https://github.com/google/oss-fuzz/pull/8234 I did an initial integration into OSS-Fuzz, and this initial fuzzer should be enough to cover a reasonable amount of the code in pyrsistent. The goal is to make the fuzzer have optimal code coverage and down the extend with e.g. property-based fuzzers.

If you would like to integrate, the only thing I need is a list of email(s) that will get access to the data produced by OSS-Fuzz, such as bug reports, coverage reports and more stats. Notice the emails affiliated with the project will be public in the OSS-Fuzz repo, as they will be part of a configuration file.

I would suggest a version of the transform method, where there is no change if a field is missing.

My motivation is, that I like the idea of pyrsistent's tranformation, but I use document databases with optional fields and subfields and I would like to keep the original state where a field or parent field is missing. (Also pydantic can use Optional fields.) Coming up with some weird lambda function instead of ny would mess up the code.

I give you some examples:

data = v(
        m(timeout="13", description="send email"),
        m(description="start database"),
    )

loose_transform(data, (ny, "timeout"), int)
# or
data.transform((ny, "timeout"), int, mode="loose")

would give back

data = v(
        m(timeout=13, description="send email"),    # The only place that changes is timeout here.
        m(description="start database"),
    )

doesn't do anything with the second element.

A second example:

data = m(
    composer=m(birth=m(place="Salzburg", year=1777), name="Felix"),
    lyrics=m(name="John"),
    arranged_by=m(name="Jack", birth=m(year=1977))
)
result = loose_transform(data, [ny, "birth", "place"], str.upper)

gives back

m(
    composer=m(birth=m(place="SALZBURG", year=1777), name="Felix"),   # The only place that changes is place here.
    lyrics=m(name="John"),
    arranged_by=m(name="Jack", birth=m(year=1977))
)

doesn't create birth/place in lyrics and arranged_by. And I don't need to use a more difficult function instead of str.upper to handle the missing fields.

I have a loose_transform implementation and unit tests as gist.

Maybe pyrsistent could use the same transform method with an additional mode keyword argument, that is "strict" by default with the current behavior, and maybe "loose" is the behavior I presented here. (I'm not a native English speaker, so I couldn't find better word than loose.)

I think that discard could work with the same way, so using discard instead of str.upper in the second example would just remove place="Salzburg" from the composer field.

https://github.com/tobgu/pyrsistent/commit/30f138142923669c9da327abe214986919d52a07 changed the way transforms work so that they add intervening nodes from the matcher which didn't previously exist.

This addressed the case in https://github.com/tobgu/pyrsistent/issues/154 and made

>>> freeze({}).transform(['foo','bar'],m())

Have the intuitive result:

pmap({'foo': pmap({'bar': pmap({})})})

Rather than

pmap({})

But I think this added an unintuitive side-effect to discard where it will add nodes.

in versions >=0.15.2:

>>> freeze({}).transform(['foo','bar'],discard)
pmap({'foo': pmap({})})

in versions <0.15.2:

>>> freeze({}).transform(['foo','bar'],discard)
pmap({})

I don't think discard should ever add anything to a PMap

This caused serious headache for me as we were using a transformation with discard to remove an empty map with a specific key if it existed, but when we bumped the version of pyrsistent we were using, that transformation started adding new empty maps one level up from the potential location of the empty map.

For example:

versions <0.15.2

>>> freeze({'baz':{'foo':{'bar':1}},'boo':{'far':2}}).transform([ny,'foo','bar'],discard)
pmap({'boo': pmap({'far': 2}), 'baz': pmap({'foo': pmap({})})}) # this is what I expect

versions >=0.15.2

>>> freeze({'baz':{'foo':{'bar':1}},'boo':{'far':2}}).transform([ny,'foo','bar'],discard)
pmap({'baz': pmap({'foo': pmap({})}), 'boo': pmap({'foo': pmap({}), 'far': 2})}) # added new PMap boo.foo

When trying the example with pylance/pyright in vscode, I get an "PylancereportMissingTypeArgument" type hint error:

e.g.:

class ARecord(PRecord):
    x = field()

PRecord is flagged with 'Expected type arguments for generic class "PRecord"'. Is this fixable somehow?