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

Device.__getstate__() is broken #86

Closed
f18m opened this issue Apr 18, 2024 · 7 comments
Closed

Device.__getstate__() is broken #86

f18m opened this issue Apr 18, 2024 · 7 comments
Assignees
Labels

Comments

@f18m
Copy link

f18m commented Apr 18, 2024

Describe the bug
The method getstate() of Device class used to return a dictionary.
Since this commit: b9e0a64#diff-75bf1e4cf04c226a1b9986d88bd9d3882dbb1e096e70920a63e243f83a9d278cR476
the implementation just returns "None".

Was that just a typo?

thanks!

@ralequi
Copy link
Collaborator

ralequi commented Apr 19, 2024

I think I don't have "getstate" method in my tests, it may be a bug/typo. Let me check in deep

@ralequi
Copy link
Collaborator

ralequi commented Apr 19, 2024

Well, probably "return None" is the issue here. Introduced 10 months ago.

Trying to remember, I think I founded some issue mapping the data. Probably left the none by a mistake.

I'm going to have a look on how to rescue it.
Just to have some background: How and for what are you using it? Do you use the all_info param?

@f18m
Copy link
Author

f18m commented Apr 19, 2024

Actually I'm just trying to get psmqtt project to work.
It uses PySMART and at this line:

https://github.com/eschava/psmqtt/blob/master/src/handlers.py#L538

it's invoking the getstate() method to get a nice dictionary to perform a lookup using a key obtained from the configuration file. In practice in the config file of that utility you list whatever SMART attribute you're interested in...

@f18m
Copy link
Author

f18m commented Apr 19, 2024

and thanks for the fast answer!

@ralequi
Copy link
Collaborator

ralequi commented Apr 19, 2024

Please, check when you are able. It should returns a valid dict now.

You may be able to install the dev version from git or from pip ( https://pypi.org/project/pySMART/1.3.1.dev8/ )

If you can provide me more info about something you are missing in that representation I may be able to provide a better output. The more info the better.

@f18m
Copy link
Author

f18m commented Apr 20, 2024

thanks @ralequi for the very fast fix!
I tested it and it works fine.
I just wonder: instead of using the 'magic' _ _getstate __() function (apparently magic is their official name, see https://peps.python.org/pep-0008/#descriptive-naming-styles), what about exposing these info also through a function like 'all_attributes_as_dict()' or 'get_state_dict()' or similar?
I think is not a good practice to invoke magicfunctions just to inspect a Python class...

thanks!

@ralequi
Copy link
Collaborator

ralequi commented Apr 22, 2024

Hi,

This is the reason behind getstate : https://docs.python.org/3/library/pickle.html#object.__getstate__ : to serialize the class under certain generic libs.

The dict function you are suggesting is in fact defined by default. And do you know which name is? Yeah, exactly: https://docs.python.org/3/library/stdtypes.html#object.__dict__

I don't like that magics functions, but while there are people using it, I can't delete them.

Thanks for your report,
I'll try to push it to an stable release soon

@ralequi ralequi closed this as completed Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants