RangeSet: support ranges with zero padding of mixed lengths (#293) #473
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
That's look great ! I did not review in details yet, waiting for a more final version, however, my 2 first comments are:
|
degremont
reviewed
Jun 27, 2022
There was a problem hiding this comment.
I'm worried by the compat breakage. I think this is too risky to release 1.9 with this RangeSet behavior change. (str vs int)
NodeSet is also behaving differently, now that the padding is not use the same way, but hopefully everybody will think this was a bug :)
Worth a beta?
) This patch adds support for mixed lengths 0-padding ranges by always using strings instead of integers in the inner set. No need to keep track of padding length per set anymore, as this information is self contained in the strings. Old behavior (single padding length supported): $ cluset -f bar002,bar01,bar0 bar[000-002] New behavior (mixed padding lengths supported): $ cluset -f bar002,bar01,bar0 bar[0,01,002] RangeSet.padding is now available as a property. In case of zero padding with mixed lengths, it returns the maximum padding length. It can also still be used to force a fixed-length padding on the set. Example: >>> r = RangeSet("0,01,002") >>> r 0,01,002 >>> r.padding 3 >>> r.padding = 4 >>> r 0000-0002 >>> r.padding = None >>> r 0-2 Older versions of RangeSet are automatically converted when unpickled. Closes cea-hpc#293 cea-hpc#442
martinetd
reviewed
Jul 1, 2022
martinetd
reviewed
Jul 1, 2022
degremont
reviewed
Jul 1, 2022
Add check when parsing bogus ranges like 01-010. This is stricter but should avoid mistake and syntax error. Make it handle properly by NodeSet.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch adds support for mixed lengths 0-padding ranges by
using sets of strings instead of integers. No need to keep
track of padding length anymore, as this information is self
contained in the strings.
$ cluset -f bar002,bar01
bar[01,002]
Deprecated RangeSet.padding which is now a noop.
Closes #293 #442