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

Alignment marker is not optional for numbers when it should #36

Closed
mraspaud opened this issue Jul 21, 2022 · 4 comments
Closed

Alignment marker is not optional for numbers when it should #36

mraspaud opened this issue Jul 21, 2022 · 4 comments

Comments

@mraspaud
Copy link
Member

While working on #35 I noticed that the alignment character is not optional for number as it should.

In [6]: parse("{foo:05d}", "00123")
Out[6]: {'foo': 123}

In [8]: parse("{foo:_>5d}", "__123")
Out[8]: {'foo': 123}

# but

In [7]: parse("{foo:_5d}", "__123")
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Input In [7], in <module>
----> 1 parse("{foo:_5d}", "__123")

File ~/usr/src/trollsift/trollsift/parser.py:457, in parse(fmt, stri, full_match)
    447 """Parse keys and corresponding values from *stri* using format described in *fmt* string.
    448 
    449 Args:
   (...)
    454 
    455 """
    456 convdef = get_convert_dict(fmt)
--> 457 keyvals = regex_formatter.extract_values(fmt, stri, full_match=full_match)
    458 for key in convdef.keys():
    459     keyvals[key] = _convert(convdef[key], keyvals[key])

File ~/usr/src/trollsift/trollsift/parser.py:368, in RegexFormatter.extract_values(self, fmt, stri, full_match)
    366 match = re.match(regex, stri)
    367 if match is None:
--> 368     raise ValueError("String does not match pattern.")
    369 return match.groupdict()

ValueError: String does not match pattern.

The format specification here: https://docs.python.org/3/library/string.html#format-specification-mini-language says > is the default for numbers, and it's already working when the fill is 0, so maybe this could be fixed? I had a quick look, but got lost in regexes...

@mraspaud mraspaud added the bug label Jul 21, 2022
@mraspaud mraspaud changed the title Alignment marker is not optional for numbers when it could Alignment marker is not optional for numbers when it should Jul 21, 2022
@djhoese
Copy link
Member

djhoese commented Jul 25, 2022

I don't think _5d is a valid format specifier:

In [1]: a = 123

In [2]: f"{a:_5d}"
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Input In [2], in <cell line: 1>()
----> 1 f"{a:_5d}"

ValueError: Invalid format specifier

In [3]: f"{a:_>5d}"
Out[3]: '__123'

What am I doing wrong?

@djhoese
Copy link
Member

djhoese commented Jul 25, 2022

The alignment is only optional if fill isn't specified it seems. I'm not sure how 0 is working.

In [17]: parse("{foo:<5d}", "  123")
Out[17]: {'foo': 123}

In [18]: parse("{foo:<5d}", "123  ")
Out[18]: {'foo': 123}

In [19]: parse("{foo:=5d}", "123  ")
Out[19]: {'foo': 123}

In [20]: parse("{foo:=5d}", "  123")
Out[20]: {'foo': 123}

In [21]: parse("{foo:5d}", "  123")
Out[21]: {'foo': 123}

@djhoese
Copy link
Member

djhoese commented Jul 25, 2022

I noticed in the docs:

When no explicit alignment is given, preceding the width field by a zero ('0') character enables sign-aware zero-padding for numeric types. This is equivalent to a fill character of '0' with an alignment type of '='.

Changed in version 3.10: Preceding the width field by '0' no longer affects the default alignment for strings.

This to me sounds like Python 3.10+ changes the behavior of the 0 fill as you're using it. So I think that means that what trollsift is doing is as expected and that your example is using an invalid format specifier (a fill without an alignment).

@mraspaud
Copy link
Member Author

ok, sorry about this then, I misunderstood the format doc. We're good here, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants