nazrulworld / fhir.resources

FHIR Resources https://www.hl7.org/fhir/resourcelist.html
https://pypi.org/project/fhir.resources/
Other
365 stars 104 forks source link

Replace the warning log with a warnings.warn instead. #138

Open janwijbrand opened 11 months ago

janwijbrand commented 11 months ago

As a result the warning is only issued once by default. A developer can choose to configure what warnings are filtered how.

Rationale: When using fhir.resources from an application framework such as fastapi, that expects to be able to use the Pydantic APIs for dict() and json() etc. the log will be "spammed" with warnings. The developer that uses the framework though has very limited control over this short of re configuring the log levels.

janwijbrand commented 11 months ago

Address https://github.com/nazrulworld/fhir.resources/issues/90 with an alternative approach. See https://github.com/nazrulworld/fhir.resources/issues/140

codecov-commenter commented 11 months ago

Codecov Report

All modified lines are covered by tests :white_check_mark:

Comparison is base (1fd6ea4) 99.97% compared to head (6b197cd) 99.97%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #138 +/- ## ======================================= Coverage 99.97% 99.97% ======================================= Files 302 302 Lines 60584 60584 ======================================= Hits 60566 60566 Misses 18 18 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nazrulworld commented 11 months ago

Thanks for your PR, it seems good but can you check Travis build for python 3.10

janwijbrand commented 10 months ago

Thanks for your PR, it seems good but can you check Travis build for python 3.10

I did have a look, but I'm a little bit at a loss here...

I'm tried run mypy locally: I get a ton of errors that I don't see on travis. Yet, the error on travis I don't see locally. The error seems to occur or at least is related to numpy. AFAICS I don't think fhir.resources itself depends on numpy. I think numpy is a (test?) dependency for orjson.

Perhaps you can trigger a travis build of the main branch manually (I don't seem to have that permission of travis) to see if the error also occurs there?

If you have any other idea, let me know!

nazrulworld commented 10 months ago

Thanks for your PR, it seems good but can you check Travis build for python 3.10

I did have a look, but I'm a little bit at a loss here...

I'm tried run mypy locally: I get a ton of errors that I don't see on travis. Yet, the error on travis I don't see locally. The error seems to occur or at least is related to numpy. AFAICS I don't think fhir.resources itself depends on numpy. I think numpy is a (test?) dependency for orjson.

Perhaps you can trigger a travis build of the main branch manually (I don't seem to have that permission of travis) to see if the error also occurs there?

If you have any other idea, let me know!

I am not 100% sure but the most provably, because of mypy version in your local, you are seeing too many errors but not in travis. for example if look at inside setup.py //"mypy" + (PY_VERSION_11_OR_LATER and ">=0.991" or "==0.812")//

I can also look that linting error, but unfortunately I am too busy right now :)

janwijbrand commented 10 months ago

I am not 100% sure but the most provably, because of mypy version in your local, you are seeing too many errors but not in travis. for example if look at inside setup.py //"mypy" + (PY_VERSION_11_OR_LATER and ">=0.991" or "==0.812")//

I can also look that linting error, but unfortunately I am too busy right now :)

I'll try to dig a bit further as well.

In the meantime I tried removing the warn() call from my PR to see if the Travis outcome for the mypy check was different. It wasnt. The Travis outcome as before, so it is not my code per sé that makes the mypy fail. I added the warn() code back again.