Fix LedgerObject.from_xrpl to only accept CamelCase JSON keys

[khancode]

Fix LedgerObject.from_xrpl to only accept CamelCase JSON keys. Currently, it accepts both CamelCase and snake_case keys which is invalid functionality.

Found this bug in this PR that adds Ledger Objects models - #663

[mvadari]

I suspect all the from_xrpl methods have this problem.

[JST5000]

I'm confused as to why this change is required. This seems like it would be a breaking change without improving the experience for users.

[mvadari]

I'm confused as to why this change is required. This seems like it would be a breaking change without improving the experience for users.

It's not how that function is supposed to be used. snake_case keys aren't the way that the XRPL represents objects and transactions, it's an xrpl-py-specific encoding. If you're using from_xrpl in that way, you should be using from_dict.

IMO it would be an experience improvement for users, because we'd be doing some additional processing to ensure that the data you're putting into that function is actually from the XRPL and not from something else. It's also more confusing that snake_case keys are supported than if they're not.

[ckeshava]

Clarification: Why are we supporting snake_case in the from_dict function while proposing the opposite in the from_xrpl function?

The two functions (from_xrpl and from_dict) appear to be doing similar things (the latter invokes the former function after performing preprocessing steps about TransactionType).

[mvadari]

Clarification: Why are we supporting snake_case in the from_dict function while proposing the opposite in the from_xrpl function?

The two functions (from_xrpl and from_dict) appear to be doing similar things (the latter invokes the former function after performing preprocessing steps about TransactionType).

That's exactly the difference between the two functions. from_xrpl converts from CamelCase to snake_case, then runs from_dict. from_dict converts the dictionary to a model object. That's why they should be doing different things.

[ckeshava]

Okay, I understand that.

By that reasoning, Transaction.from_xrpl should not accept non-CamelCase encoding of JSON inputs. Am I right?

I couldn't find equivalent functions in the Typescript (xrpl.js) codebase. Are we obligated to maintain consistency between the libraries?

I'd prefer to maintain adherence to the XRPL encoding formats. To that end, why are we supporting snake_case encoding at all? I know that it would break backwards compatibility now, but I'd like to know the original reason for the support.

[mvadari]

By that reasoning, Transaction.from_xrpl should not accept non-CamelCase encoding of JSON inputs. Am I right?

Yes, this issue should really be about all from_xrpl methods.

I couldn't find equivalent functions in the Typescript (xrpl.js) codebase. Are we obligated to maintain consistency between the libraries?

There are language-specific differences between the libraries. If something works better in a certain way in JS vs. Python, or if the norms are different for that language, then it'll be different. Another example of a difference based on norms in a language are how we have class models for Python but just TypeScript types (instead of classes) for JS.

I'd prefer to maintain adherence to the XRPL encoding formats. To that end, why are we supporting snake_case encoding at all? I know that it would break backwards compatibility now, but I'd like to know the original reason for the support.

It's more Pythonic to use snake_case.

[ckeshava]

ok thanks, that's helpful