8000 Serialize np.float64 as float · Issue #257 · Azure/msrest-for-python · GitHub
[go: up one dir, main page]
Skip to content

Serialize np.float64 as float #257

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.

Sign up for GitHub

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

Jump to bottom
Open
mzat-msft opened this issue Nov 16, 2022 · 2 comments
Open

Serialize np.float64 as float #257

mzat-msft opened this issue Nov 16, 2022 · 2 comments
Assignees
@lmazuel

Comments

@mzat-msft
Copy link
mzat-msft commented Nov 16, 2022 •
edited
Loading

On my machine np.float64 are the same as float, so I would expect that they are serialized in the same way. However, this is not the case as the logic in

msrest-for-python/msrest/serialization.py

Line 944 in af41991

def serialize_object(self, attr, **kwargs):
infer the type of the object's attribute using type rather than isinstance.

Do you think it makes sense to treat classes that are subclasses of basic types as basic types in the serialization process?

Here is a minimal working example of what I'm referring to. Note that np-float is cast to a str instead of a float.

import msrest.serialization
import numpy as np


class ModelWithNpFloat(msrest.serialization.Model):
    _attribute_map = {
        "state": {"key": "state", "type": "object"},
    }

    def __init__(self, state):
        self.state = state


model = ModelWithNpFloat(state={'np-float': np.float64(100)})
print(model.serialize())  # {'state': {'np-float': '100.0'}}
@lmazuel
Copy link
Member
lmazuel commented Nov 16, 2022

This is a good point, I'd love to see our serialization supports numpy by default. We won't do it in msrest though as we are deprecating this lib to a new system, recent SDKs don't even install msrest anymore. I'll keep this issue open until we update the new system with it.

@lmazuel lmazuel self-assigned this Nov 16, 2022
@mzat-msft
Copy link
Author
mzat-msft commented Nov 17, 2022 •
8EB4 edited
Loading

Thanks for your feedback!
I noticed that the package was deprecated only after opening the issue, but decided to keep the issue open for our reference as we have a package that depends on this issue.

Actually, I think that my suggestion is more connected to python's duck-typing. The issue with numpy is just one manifestation.
What I mean is that for cases where a class is behaving as a built-in (i.e. being a subclass of a basic type) it might make sense to serialize them in the same way.
In light of this I understand that the title of my issue is misleading.

mzat-msft added a commit to microsoft/bonsai-gym that referenced this issue Nov 17, 2022
@mzat-msft
Check if state is valid more strictly
9f95f0e 
Due to Azure/msrest-for-python#257 we need to check
against ``type`` instead of ``isinstance`` otherwise ``numpy.float_`` will
pass the check but fail at serialization.
@mzat-msft mzat-msft mentioned this issue Dec 8, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lmazuel lmazuel

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@lmazuel @mzat-msft
0