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

Public transform #180

Closed
jannikluhn opened this issue Oct 27, 2019 · 2 comments
Closed

Public transform #180

jannikluhn opened this issue Oct 27, 2019 · 2 comments

Comments

@jannikluhn
Copy link

I'm trying to build a datastructure which internally uses a PVector but adds some functionality. I would like it to expose (almost) the same interface as a PVector. For most methods this is trivial (just a little verbose): I can call the corresponding method on my internal PVector and add my custom code. The only problem is transform: It seems like using pyrsistent._transformations.transform should just work, but I have two concerns:

  • transform is technically not public
  • it is unclear to me what the exact interface is that MyPVector and the corresponding evolver have to support to be compatible with transform

So my questions are:

  • Can I rely on transform being stable? Is there a reason why it's not imported into the main namespace (if not I'd gladly create a PR)?

  • By reading the code, transform seems to use

    • MyPVector.items or MyPVector.__iter__
    • MyPVector.__getitem__
    • MyPVector.evolver
    • MyEvolver.__delitem__
    • MyEvolver.__setitem__
    • MyEvolver.persistent()

    Have I missed anything?

@tobgu
Copy link
Owner

tobgu commented Oct 27, 2019

The reason transform is not public is exactly the things you mention above. It makes alot of assumptions about things that should be present and work in a particular way on a "transformable" object.

The internals of transform has been very stable for many years and I don't expect them to change in the near future. There are a couple of feature requests related to transform (#53, #89, #95) that could perhaps alter the behaviour. Most of them have been open for a long time though without anyone acting on them. I personally don't have time or interest to work on them ATM.

If I were in your spot I'd probably make use of the non-public function and make sure I had sufficient test coverage for the behaviour I want to support to notice any changes in the behaviour of transform. As stated above I expect it to be very stable but you never know... I don't think I'd like to commit to the exact interface required to support transforms at this time but perhaps it's not such a big deal. I've never really though about transform that way before.

@jannikluhn
Copy link
Author

Fair enough, this sounds reasonable to me. Thank you for the assessment.

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