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

Allow raw data to be returned #27

Open
nano-shino opened this issue May 11, 2022 · 3 comments
Open

Allow raw data to be returned #27

nano-shino opened this issue May 11, 2022 · 3 comments
Labels
enhancement New feature or request

Comments

@nano-shino
Copy link

nano-shino commented May 11, 2022

This has been bugging me for a while but can you include the raw json data in the returned pydantic model? Sometimes the assumption that you used to parse the json isn't correct, and I would have to hack into the library to get the original json to parse it myself.

Here's an example: The real-time notes API returns 'transformer': {'obtained': True, 'recovery_time': {'Day': 0, 'Hour': 5, 'Minute': 0, 'Second': 0, 'reached': True}, 'wiki': ''}. Mihoyo only returns the biggest time unit. In this case it rounds down to 5 hours so even if its 5 hours and 12 minutes it still returns 5 hours. The remaining_transformer_recovery_time as it's currently implemented gives the wrong impression that it will recover at this exact time. Meanwhile if I have the original json, I can display something like "parametric transformer will recover in 5 hours...", "parametric transformer will recover in 21 minutes..."

This is just one example. I have come across a few cases where the raw json is greatly appreciated, like the fields that you haven't yet added (e.g. data["transformer"]["obtained"])

@thesadru
Copy link
Owner

thesadru commented May 11, 2022

I'm all for making patching easier but I'm not sure if this is truly the best approach. Storing raw values should be possible with a root validator in the base class so I can do that but I'd rather allow the user to make their own models if they so wish.

For this specific case I think I should store the time as a timedelta instead of a datetime which should allow for easier usage.

The Transformer will return None if not obtained as opposed to giving 0 remaining time so that's not really a problem.

There is a TODO list just in case.

@thesadru thesadru added the enhancement New feature or request label May 11, 2022
@thesadru
Copy link
Owner

This will be fixed by removing the pydantic dependency and switching to a custom model library. Development is slow but it's being worked on.

@nano-shino
Copy link
Author

Thanks! That's great to hear. I like the idea of pretty Python models with pydantic but it can be restricting sometimes. Personally I prefer genshin.py to remain a barebone library or at least expose the basic hoyolab APIs so I can build on other things on it.
The custom model library looks very promising. Can't wait to see it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants