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

Fix generated code of the json provider with PreferDictionaries when values are arrays #1476

Merged
merged 2 commits into from
Mar 7, 2023

Conversation

mlaily
Copy link
Contributor

@mlaily mlaily commented Mar 1, 2023

Before this PR, the generation is wrong:

image

image

This is because the records inferred originally in the arrays are normally supposed to have the name of the property declaring them (123 and 789 in the screenshots), and two similar records with a different name are not merged.

This problem was taken care of by the original PreferDictionaries implementation when values are records (by dropping the record's name), but not when the values are arrays of records!

After this PR, the names of records nested in an array are also dropped, which yields the expected result, with records properly merged in a single type:

image

(PR of the original implementation for reference: #1430)


Note: This PR contains only 2 commits, but is currently on top of the rearrange-project-files branch, from #1475 — I will rebase it on master when #1475 is merged.

EDIT: rebased and ready!

@mlaily
Copy link
Contributor Author

mlaily commented Mar 1, 2023

The build is failing because the worldbank api is apparently down -_-

Error: D:\a\FSharp.Data\FSharp.Data\tests\FSharp.Data.Tests\WorldBankProvider.fs(16,5):
error FS3021: Unexpected exception from provided type 'FSharp.Data.WorldBankData+ServiceTypes+Indicators' member 'GetMethods':
 The type provider 'ProviderImplementation.WorldBankProvider' reported an error: Failed to request 'http://api.worldbank.org/v2/indicator?per_page=1000&format=json&page=3'.
 Error: System.Net.WebException: Timeout exceeded while getting response
 ---> System.TimeoutException: The operation has timed out.

@mlaily mlaily force-pushed the fix-json-dictionaries branch from 898dbee to 47122e2 Compare March 5, 2023 22:08
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.

Thank you! Also appreciate the extensive test cases.

@cartermp cartermp merged commit 09d38f7 into fsprojects:main Mar 7, 2023
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.

2 participants