Small fixes and get_mindat_json refactor.

I will go through and change it from error messages to print statements to make them suggestions. I have done the testing for and "", both work. And for the api changes I have tested endpoints that usually gives some trouble as well as others through normal testing, I will go through and test all of them officially but so far I haven't had any issues.

From: Jiyin Zhang @.> Sent: Thursday, May 16, 2024 3:23 PM To: ChuBL/OpenMindat @.> Cc: CorylC @.>; Author @.> Subject: Re: [ChuBL/OpenMindat] Small fixes and get_mindat_json refactor. (PR #15)

@ChuBL requested changes on this pull request.

Thank you for your updates! Please take a look at the comments and make the necessary improvements. Thanks.

On openmindat/geomaterials.pyhttps://github.com/ChuBL/OpenMindat/pull/15#discussion_r1604118365:

I appreciate your effort on input validations. However, I have some comments. Except for type issues, I suggest we do not use hard input validation with a list of valid options but only print a message indicating that the inputs might be wrong and attaching with a corresponding redoc link. You may refer to my modification on the country function of localities.py for references. This modification will allow users to input "invalid" params because the allowed input might change according to API developments. The hard-coded restriction is not a good idea for a continuously developing API.

In openmindat/localities.pyhttps://github.com/ChuBL/OpenMindat/pull/15#discussion_r1604121431:

@@ -191,9 +193,12 @@ def expand(self, EXPAND_FIELDS): if invalid_options: raise ValueError(f"Invalid EXPAND_FIELDS: {', '.join(invalid_options)}\nEXPAND_FIELDS must be one or more of the following: {', '.join(valid_options)}")

expand_fields = EXPAND_FIELDS
Changes to "" and from a list to a string since this is what the param will read without failing.
expand_fields = [w.replace('', '""') for w in expand_fields]

I am wondering if the initial user input is a "" and your code replaces it with """". Will the API server still recognize and process it? Again, do not raise Value Error for the "invalid" inputs, but printing it as a soft warning would be better.

On openmindat/mindat_api.pyhttps://github.com/ChuBL/OpenMindat/pull/15#discussion_r1604126739:

Your modification seems reasonable. Since this function has a broad impact on other classes, I assume you have tested through most of the cases and proven the modification to be functional. Otherwise, please revise it until it passes the tests.

— Reply to this email directly, view it on GitHubhttps://github.com/ChuBL/OpenMindat/pull/15#pullrequestreview-2061999929, or unsubscribehttps://github.com/notifications/unsubscribe-auth/BCW4I3N7PCPP7LZY6RQNKWLZCUWXVAVCNFSM6AAAAABH3AP2L2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDANRRHE4TSOJSHE. You are receiving this because you authored the thread.Message ID: @.***>

ChuBL / OpenMindat

Small fixes and get_mindat_json refactor. #15

Changes to "" and from a list to a string since this is what the param will read without failing.