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

Feature Request: __getattr__ #116

Closed
rwijtvliet opened this issue Mar 21, 2021 · 6 comments
Closed

Feature Request: __getattr__ #116

rwijtvliet opened this issue Mar 21, 2021 · 6 comments

Comments

@rwijtvliet
Copy link

rwijtvliet commented Mar 21, 2021

Your system
3.9

What version of libgphoto2 have you installed?
2.5.25

How have you installed (or attempted to install) python-gphoto2?
yes

Your problem

When using the package, I find I'm using the get_child_by_name attribute very often...

cfg = cam.get_config()
current_iso = cfg.get_child_by_name('iso').get_value()
cfg.get_child_by_name('iso').set_value('100')

...where a more object-oriented / user-friendly interface might be

cfg.iso = '100'

If there is a reason you haven't written it like this (staying close to the gphoto2 API maybe?), then you can ignore this request, of course.

I have tried to implement it myself, but ran into an issue I'm not sure I've seen before, at the first step (exposing the children as attributes). Still, it might be helpful to you, so I'll include it here. This should have allowed access to the widget with cfg.iso; setting its value would have been the next step.

# Make children available as attributes by their name.
gphoto2.widget.CameraWidget.__getattr__ = lambda self, name: self.get_child_by_name(name)
# Gives an AttributeError: type 'gphoto2.widget.CameraWidget' has no attribute '__getattr__'
# which is weird, as I'm trying to *create/define/set* the attribute __getattr__, not access it.
@jim-easterbrook
Copy link
Owner

I hadn't thought about doing it like this. It would add to the interface, and in general I don't like to combine libgphoto2 functions into something new.

I would call the get_child_by_name method once to get that config item, then call get_value and set_value as required after that. It might make sense to use these for pseudo attribute access - I think SWIG makes that fairly easy. So you might have:

cfg = cam.get_config()
current_iso = cfg.get_child_by_name('iso')
value = current_iso.value
current_iso.value = '100'

I'm not surprised your attempt to modify the gphoto2.widget.CameraWidget class fails. SWIG generated stuff is not pure Python!

@jim-easterbrook
Copy link
Owner

I've been doing some experimenting with SWIG's "attribute.i" library, but not achieving any satisfactory result. The situation is complicated by the overloading of get_value and set_value to cope with three different types of data - int, float, and str. SWIG's idea of attribute access doesn't seem to be able to handle that.

Given that attribute style access isn't essential, and it breaks away from the libgphoto2 way of doing things, I don't plan to pursue this any further.

jim-easterbrook added a commit that referenced this issue Mar 26, 2021
See feature request #116. This is a bit kludgy, and is read-only so far,
but it seems to work and should be faster than calling
gp_widget_get_value or CameraWidget.get_value
@rwijtvliet
Copy link
Author

Alright, thanks for looking into it. I'm closing the issue then.

@jim-easterbrook
Copy link
Owner

Never say never. I'm keeping this in mind for the next release.

jim-easterbrook added a commit that referenced this issue Apr 20, 2021
Another step towards implementing request #116.
@jim-easterbrook
Copy link
Owner

Having successfully done read-only implementation of 3 attributes I don't think this is worth taking any further. I take your point about it being more Pythonic, but it hides the fact that there's a libgphoto function call behind every attribute access. I'll take the attribute access stuff out again.

jim-easterbrook added a commit that referenced this issue Apr 21, 2021
This was discussed in feature request #116. Adding attribute access is
possible but messy.
@rwijtvliet
Copy link
Author

Alright - thanks for taking the time to look into it

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

No branches or pull requests

2 participants