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

Add EnsureMappings constraint for input with several key-value pairs #289

Closed
wants to merge 3 commits into from

Conversation

adswa
Copy link
Member

@adswa adswa commented Mar 6, 2023

Fixes #283

This adds an EnsureMappings() constraint, which closely mimics EnsureMapping(), but handles input consisting of multiple key value pairs.
I'm not sure if I should take into account the fact that a mapping could get really long, though. I think I could base at least parts of this on itertools methods - what do you think about that?

Update/Edit: Based on discussions in #290 and here, I have reworked this PR. I would be grateful if someone could check if it contains anything of use - I got entangled a bit too deep have currently lost orientation as for "why" the changes are needed. I'll make attempt to support some of the stuff that meta-add is doing, and then I might get the full picture again.

The changes I added now are:

  • I realized that EnsureMapping can not easily be replaced because EnsureParameterConstraint relies on it. The remainder of what I have implemented contains at least parts of its functionality, though, so I'm not sure if my PR adds anything of value.
  • Instead of touching EnsureMapping, I have for now just added an additional EnsureNTuple Constraint that was supposed to replace it in EnsureMapping -> Ensure2Tuple #290. EnsureNTuple has the ability to take a list of Constraints, match the constraints to the input values, and returns the outcome as a tuple. The difference between the existing EnsureTupleOf and EnsureNTuple is the ability to define different constraints for different items.
  • I have added the functionality to split strings by a delimiter into the base class EnsureIterableOf in an attempt to reduce code duplication (as EnsureMapping contained this functionality first). EnsureNTuple derives from EnsureIterableOf to make use of string splitting; however that decision should receive a review because its not really elegant - I needed to initialize the super with a NoConstraint() to not violate its parameters.
  • I have added the Constraint EnsureDict, which returns input as a dict and can apply constraints separately to keys and values. Its similar to the previous attempts of EnsureMappings, but does not do any string splitting. This constraint, too, overlaps with what EnsureMapping does, so I'm not sure if it has inherent value.

Because I'm quite uncertain about the future of any of these changes, I'm putting the PR into draft mode.

@adswa adswa requested a review from mih as a code owner March 6, 2023 09:43
@adswa
Copy link
Member Author

adswa commented Mar 6, 2023

Note to self because I'm currently working on another branch: Needs an input in init

@mih
Copy link
Member

mih commented Mar 6, 2023

Protocol of recent chat: Better not continue the mistake of EnsureMapping (issue will come), and implement EnsureDict. it would take any iterable, it would support an optional item constraint that can check/coerce each item into a key-value-pair (not a dict, just a tuple), and then send the global iterable to the dict constructor.

@adswa adswa marked this pull request as draft March 7, 2023 12:03
@mih
Copy link
Member

mih commented Jul 27, 2023

This has been in draft mode for a long time, and has now accumulated conflicts. I am closing this PR now as part of a cleanup run. Feel free to reopen if there is still desire. Sorry.

@mih mih closed this Jul 27, 2023
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

Successfully merging this pull request may close these issues.

Create an EnsureMappings() constraint
2 participants