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

Added InferDictionariesFromRecords flag to JSON TP #1430

Merged
merged 3 commits into from
Feb 28, 2022

Conversation

nikoyak
Copy link
Contributor

@nikoyak nikoyak commented Feb 18, 2022

JSON TP always projects json record fields into the properties
of generated types. Sometimes it is not desirable. The added
flag will allow to infer dictionaries from the records if the names
of all the fields of similar records in the provider's training data
are inferred to the same non-string primitive type (by type inference
rules). The flag is not set by default for backwards compatibility.

The current implementation makes it meaningless to process such data in the JSON Provider:

{ "sales": { "2022-01-13": { "count": 3 }, "2022-01-23": { "count": 5 }, "2022-01-28": { "count": 4 }, "2022-02-01": { "count": 5 }, "2022-02-07": { "count": 1 }, "2022-02-13": { "count": 2 } } }

With this PR, for example, it is easy to find out from the above data how many sales were in February:

type TP = JsonProvider<"""{ "sales": { "1970-01-01": { "count": 100 }}}""", InferDictionariesFromRecords = true>
let s = """{ "sales": { "2022-01-13": { "count": 3 }, "2022-01-23": { "count": 5 }, "2022-01-28": { "count": 4 }, "2022-02-01": { "count": 5 }, "2022-02-07": { "count": 1 }, "2022-02-13": { "count": 2 } } }"""
TP.Parse(s).Sales.Items
|> Seq.filter (fun (k, _) -> k.Month = 2)
|> Seq.sumBy (fun (_, v) -> v.Count)

In addition to the Items: (InferedKey * InferedValue) seq property specified in the code above, the generated dictionary-like type contains the Count, IsEmpty, Keys, Values properties, the Item indexed property, and the ContainsKey, TryFind methods, similar to the members of FSharp.Collections.Map

JSON TP always projects json record fields into the properties
of generated types. Sometimes it is not desirable. The added
flag will allow to infer dictionaries from the records if the names
of all the fields of similar records in the provider's training data
are inferred to the same non-string primitive type (by type inference
rules). The flag is not set by default for backwards compatibility.
Copy link
Collaborator

@cartermp cartermp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Took a look, I think this is a great feature to add. Code looks reasonable.

I'd like to spend a little time mulling it over, though. @dsyme would like to get your take as well.

src/Json/JsonGenerator.fs Outdated Show resolved Hide resolved
- removed extra pipe
! added missing newline at end of the files
+ code reusing
@dsyme
Copy link
Contributor

dsyme commented Feb 24, 2022

Suggest PreferDictionaries instead of InferDictionariesFromRecords

src/Json/JsonGenerator.fs Outdated Show resolved Hide resolved
@dsyme
Copy link
Contributor

dsyme commented Feb 24, 2022

I left a couple of suggestions, but otherwise marked as approved, the code looks great

- dropped overused lambda expressions
* renamed the InferDictionariesFromRecords flag into PreferDictionaries
@nikoyak
Copy link
Contributor Author

nikoyak commented Feb 25, 2022

done

@cartermp
Copy link
Collaborator

Thanks!

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.

3 participants