crdoconnor / strictyaml

Type-safe YAML parser and validator.
https://hitchdev.com/strictyaml/
MIT License
1.44k stars 60 forks source link

Parse hexadecimal integers #105

Open wrobell opened 4 years ago

wrobell commented 4 years ago

YAML supports multiple integer representations

https://yaml.org/type/int.html

for example - 0x77 parses as [119].

StrictYAML raises validation error

In [1]: import strictyaml
In [2]: strictyaml.load('- 0x77', strictyaml.Seq(strictyaml.Int()))
...
YAMLValidationError: when expecting an integer
found arbitrary text
  in "<unicode string>", line 1, column 1:
    - '0x77'
     ^ (line: 1)

This ticket is about parsing hexadecimal integers as they are quite common.

crdoconnor commented 4 years ago

Would a HexInt validator serve your purposes?

On Tue, 19 May 2020, 00:50 wrobell, notifications@github.com wrote:

YAML supports multiple integer representations

https://yaml.org/type/int.html

for example - 0x77 parses as [119].

StrictYAML raises validation error

In [1]: import strictyaml In [2]: strictyaml.load('- 0x77', strictyaml.Seq(strictyaml.Int())) ... YAMLValidationError: when expecting an integer found arbitrary text in "", line 1, column 1:

  • '0x77' ^ (line: 1)

This ticket is about parsing hexadecimal integers as they are quite common.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/crdoconnor/strictyaml/issues/105, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOJKNPJJPI6WTZIEGADMLTRSHCSRANCNFSM4NEQJWFA .

wrobell commented 4 years ago

For my purposes Int() | HexInt() would be more than enough. But I think that general expectation would be to follow YAML specification, i.e. Int() (or YAMLInt()?) should allow both forms.

crdoconnor commented 4 years ago

The reason this project exists is because I wanted to strip out extraneous features from the YAML spec and pare it down to something much simpler and less fragile.

I'm happy to accommodate this use case with Int() | HexInt() though.

On Tue, 19 May 2020, 10:42 wrobell, notifications@github.com wrote:

For my purposes Int() | HexInt() would be more than enough. But I think that general expectation would be to follow YAML specification, i.e. Int() (or YAMLInt()?) should allow both forms.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/crdoconnor/strictyaml/issues/105#issuecomment-630710397, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOJKNMOATBQVNNVBQZ6XYDRSJIAHANCNFSM4NEQJWFA .

wrobell commented 4 years ago

IMHO, the question is: is there a valid use case for schema to allow only integers with specific base in the schema?

crdoconnor commented 4 years ago

Potentially, yes. 0x1a is not really very readable for non technical people. Use of hex is quite a niche thing.

On Mon, 1 Jun 2020, 10:42 wrobell, notifications@github.com wrote:

IMHO, the question is: is there a valid use case for schema to allow only integers with specific base in the schema?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/crdoconnor/strictyaml/issues/105#issuecomment-636742953, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOJKNMUQDK4JXWCCQYR2FLRUNZWXANCNFSM4NEQJWFA .

eulores commented 3 years ago

Hello, I run into the same issue. While transpiling a specification published as PDF to parseable YAML, I had to rewrite 0xFFFF (found verbatim in the PDF specification) to 65535. IMO, non-technical people would appreciate if there is a one-to-one correlation between the specs and the YAML code.

Having said that, I'll be quite happy once the proposed Int()|HexInt() solution is implemented :-)

eulores commented 3 years ago

This is a dirty patch that I use in my code to let Int() also parse hexadecimal encoded integers, such as 0xFF.

from strictyaml import Int

def validate_hexint(self, chunk):
  val = chunk.contents
  try:
    return int(val.replace("_", ""), 0)
  except:
    chunk.expecting_but_found("when expecting an integer")

Int.validate_scalar = validate_hexint
jnichols0 commented 2 years ago

Are there plans for a HexInt validator to be part of strictyaml? I have implemented it myself but I'm curious if I wont have to in the future or shouldn't even right now.

crdoconnor commented 2 years ago

Yeah, I can absolutely do this. Thanks for reminding me.

Can you share me yours?

On Fri, 1 Oct 2021, 00:52 James Nichols, @.***> wrote:

Are there plans for a HexInt validator to be part of strictyaml? I have implemented it myself but I'm curious if I wont have to in the future or shouldn't even right now.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/crdoconnor/strictyaml/issues/105#issuecomment-931785009, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOJKNO3K6HARM4FY6BWPNTUETZ3FANCNFSM4NEQJWFA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

jnichols0 commented 2 years ago

Yes definitely, mind if I submit a PR? I would love to "officially" contribute.

crdoconnor commented 2 years ago

Not at all, I'd love that.

Please bear in mind that the tooling around the project might have issues due to some code rot.

On Fri, Oct 1, 2021 at 11:31 PM James Nichols @.***> wrote:

Yes definitely, mind if I submit a PR? I would love to "officially" contribute.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/crdoconnor/strictyaml/issues/105#issuecomment-932613924, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOJKNMO6QNCJW5Y5BG7UNTUEYZEDANCNFSM4NEQJWFA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

crdoconnor commented 2 years ago

This is something I will try to fix in the next few days.

On Sun, 3 Oct 2021, 17:10 Colm O'Connor, @.***> wrote:

Not at all, I'd love that.

Please bear in mind that the tooling around the project might have issues due to some code rot.

On Fri, Oct 1, 2021 at 11:31 PM James Nichols @.***> wrote:

Yes definitely, mind if I submit a PR? I would love to "officially" contribute.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/crdoconnor/strictyaml/issues/105#issuecomment-932613924, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOJKNMO6QNCJW5Y5BG7UNTUEYZEDANCNFSM4NEQJWFA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

jnichols0 commented 2 years ago

What I threw together earlier was kind of hacky so I started over.

Take a look if you like: https://github.com/jnichols0/strictyaml/tree/hexint

I'll work on conforming to the requested PR format and adding a story test.

jnichols0 commented 2 years ago

Submitted a PR, made a story to test it.

crdoconnor commented 2 years ago

Thanks! I'll take a look a little later on.

On Sat, 9 Oct 2021, 22:49 James Nichols, @.***> wrote:

Submitted a PR, made a story to test it.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/crdoconnor/strictyaml/issues/105#issuecomment-939366411, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOJKNLVFC6AQMOW7YIRCUDUGC2GLANCNFSM4NEQJWFA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

crdoconnor commented 2 years ago

LGTM. I'll merge it and release a new version a bit later.

On Sun, 10 Oct 2021, 13:33 Colm O'Connor, @.***> wrote:

Thanks! I'll take a look a little later on.

On Sat, 9 Oct 2021, 22:49 James Nichols, @.***> wrote:

Submitted a PR, made a story to test it.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/crdoconnor/strictyaml/issues/105#issuecomment-939366411, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOJKNLVFC6AQMOW7YIRCUDUGC2GLANCNFSM4NEQJWFA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

jnichols0 commented 2 years ago

I realized HexInt, as implemented, doesn't support signed expressions. Python evaluates -0x1 to -1 but -0x1 doesn't pass validation for HexInt() | Int() which seems wrong. I'll submit a PR updating the regex if you agree.

crdoconnor commented 2 years ago

Yes, of course. Thank you.

On Tue, 19 Oct 2021, 21:43 James Nichols, @.***> wrote:

I realized HexInt, as implemented, doesn't support signed expressions. Python evaluates -0x1 to -1 but -0x1 doesn't pass validation for HexInt() | Int() which seems wrong. I'll submit a PR updating the regex if you agree.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/crdoconnor/strictyaml/issues/105#issuecomment-947089919, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABOJKNLWFPS4SW275XEUH63UHXJ7NANCNFSM4NEQJWFA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.