ChuBL / OpenMindat

A Python package for the OpenMindat API.
https://pypi.org/project/openmindat/
Apache License 2.0
4 stars 2 forks source link

Merge Request. #2

Closed CorylC closed 7 months ago

CorylC commented 7 months ago

Merge Request between my main repository and the projects. New features include:

CorylC commented 7 months ago

You're welcome! I did want to check in regarding the type hinting in the parameters. I figured it would be simpler to add type checking when it was needed in those id methods. If you would prefer I can change all of the parameters to type hinting.

Best,       Cory.


From: Jiyin Zhang @.> Sent: Wednesday, March 6, 2024 10:12 PM To: ChuBL/OpenMindat @.> Cc: CorylC @.>; Author @.> Subject: Re: [ChuBL/OpenMindat] Merge Request. (PR #2)

@ChuBL approved this pull request.

Thank you for your timely modification!

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

ChuBL commented 7 months ago

Type hinting is a good programming style but optional. The developing progress for this project is almost done. If that's okay with you, let's keep it in mind for future Python projects. I am going to test the functions you added. I will sync your work to the pypi if they work well. Thank you for your hard work!

CorylC commented 7 months ago

I forgot to mention that I couldn’t get the locobject endpoint to work, every id I tried gave me ""error": "Object not found"".


From: Jiyin Zhang @.> Sent: Wednesday, March 6, 2024 10:24:49 PM To: ChuBL/OpenMindat @.> Cc: CorylC @.>; Author @.> Subject: Re: [ChuBL/OpenMindat] Merge Request. (PR #2)

Type hinting is a good programming style but optional. The developing progress for this project is almost done. If that's okay with you, let's keep it in mind for future Python projects. I am going to test the functions you added. I will sync your work to the pypihttps://pypi.org/project/openmindat/ if they work well. Thank you for your hard work!

— Reply to this email directly, view it on GitHubhttps://github.com/ChuBL/OpenMindat/pull/2#issuecomment-1982539850, or unsubscribehttps://github.com/notifications/unsubscribe-auth/BCW4I3PI632SWZMLFFWTVTTYXAB3DAVCNFSM6AAAAABEBD46E2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBSGUZTSOBVGA. You are receiving this because you authored the thread.

ChuBL commented 7 months ago

It's possible that the server is to blame. It is surprising that you didn't encounter the Server 500 Error during development, haha.

CorylC commented 6 months ago

I have fixed these issues in the latest update to my personal repository.


From: Jiyin Zhang @.> Sent: Friday, March 22, 2024 4:31 PM To: ChuBL/OpenMindat @.> Cc: CorylC @.>; Author @.> Subject: Re: [ChuBL/OpenMindat] Merge Request. (PR #2)

@ChuBL commented on this pull request.


In openmindat/countries.pyhttps://github.com/ChuBL/OpenMindat/pull/2#discussion_r1536371236:

@@ -0,0 +1,206 @@ +from . import mindat_api + + +class CountriesRetriever:

Second revision:

  1. The endpoint to this class is countries_list. Therefore, please consider using another class name, e.g., "CountriesListRetriever".
  2. The querying result for this class is incomplete: it only returns one page of items. Please consider programming a specific function for obtaining the countries in a for loop using your new function of getting the list results. This would be a tentative solution for users to obtain the entire countries list.

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