Skip to content

Add a default attribute to properties#273

Open
rwb27 wants to merge 4 commits intomainfrom
property-default
Open

Add a default attribute to properties#273
rwb27 wants to merge 4 commits intomainfrom
property-default

Conversation

@rwb27
Copy link
Collaborator

@rwb27 rwb27 commented Mar 2, 2026

This PR adds a default attribute to properties, where it can be defined. Currently, this applies only to DataProperty and DataSetting as we don't have a neat way to define a default for functional properties. The default may be accessed in two ways:

  1. As thing.properties['name'].default (from Python)
  2. As the default key in the PropertyAffordance in the Thing Description.

I have also added a reset method, which resets the property to that value, if it exists.

If you attempt to reset or get the default of a FunctionalProperty we raise a new FeatureNotAvailable exception which I hope makes it clear.

The last step is to add a way to reset properties over HTTP. I am minded to make that a separate PR, because:

  • The current (standard compliant) PR already makes it possible to reset, because the default value is exposed in the TD.
  • There's no WoT standard way to reset properties to default, as far as I know.

I can see three sensible ways to expose resetting over HTTP:

  • Add an extra POST endpoint: POST http://my.server:5000/my_thing/my_property/reset
  • Add a DELETE endpoint at the property URL DELETE http://my.server:5000/my_thing/my_property
  • Add an action that takes the property name as a parameter: POST http://my.server:5000/my_thing/reset_property with payload my_property.

Of these, I think we discussed DELETE before and nobody liked it. I think the first one is clearest if we ignore WoT standards. The action is fine, but feels ugly, and currently it involves a lot more HTTP requests than it really needs, as the action would need polling to completion to find out whether it was successful.

I'd be happy to merge this in its present state (i.e. without extra endpoints to reset) and we can discuss what to do about resetting. If we get a consensus quickly, I have also written the code to do resets, but not yet tested it.

Closes #257

rwb27 added 3 commits March 2, 2026 12:14
This works on DataProperty and raises a specific exception for FunctionalProperty.

It is not yet exposed over HTTP, but is tested in Python.
This will fail if the default won't validate. We probably want a check for this when the `Thing` is defined, as otherwise we will get annoying and unclear errors after the server has started.
@rwb27 rwb27 requested a review from julianstirling March 2, 2026 14:52
@rwb27 rwb27 force-pushed the property-default branch from 44c4d09 to 6b79288 Compare March 2, 2026 15:05
@barecheck
Copy link

barecheck bot commented Mar 2, 2026

Barecheck - Code coverage report

Total: 96.52%

Your code coverage diff: 0.00% ▴

Uncovered files and lines
FileLines
src/labthings_fastapi/properties.py504, 794, 798, 821-824, 896, 915, 1166

Copy link
Contributor

@julianstirling julianstirling left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried to go through it. I think it is going to be annoying to have some property resetable and others not so I would really prefer FunctionalProperties to be resettable as quote a few of our properties have side effects and are still functional.

This being said it isn't too clear to me how the extra methods for a descriptor are defined hence some of the questions that go deeper.

"""


class FeatureNotAvailable(NotImplementedError):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is generally recommended for all exceptions to end with Error. On the OFM there is a Ruff rule that enforces this.

)


class FunctionalProperty(BaseProperty[Owner, Value], Generic[Owner, Value]):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice if FunctionalProperty could allow setting a reset method as:

@myprop.reset
def _reset_myprop(self):
    """Resetting my property."""
    self._myprop = 1066

or even

myprop.default = 1066

Copy link
Contributor

@julianstirling julianstirling Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually would it be possible to pass an an argument to the decorator?

_my_property=1066

@lt.property(default=1066):
def my_property(self):
    ...

I suppose could even then be:

_my_property=1066

@lt.property(default=_my_property):
def my_property(self):
    ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a reset function for a functional property makes sense to me, and I've deliberately left it so that this should be quite simple to implement. I was trying to keep this PR, so I've not added that feature in here, though if I'm adding more commits to add a reset endpoint, I could also add resetting for functional properties.

That said, if you've already reviewed this, it would be not unreasonable to merge it, and then add reset for functional properties in a future one.

Having a default for functional properties feels slightly odd to me, though as I write it I can't fully explain why. I think it's because the default for a DataProperty is the value it starts at - the same isn't necessarily true for a FunctionalProperty.

I really need to add side-effects! But that is an entirely separate PR...

def _set_myprop(self, val: int) -> None:
self._myprop = val

myprop.readonly = True # Prevent client code from setting it
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, quite deep into the weed trying to follow this PR. Is this implemented? I don't see where it is defined, but I find eveything about these descriptors really really confusing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I have checked it, and it works, I don't think it has a test. And I don't actually see how it is defined.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure what you mean by "it" here... do you mean setting myprop.readonly? If so, I was fairly sure there was at least one test for that. It's implemented by exactly the same code that implements

myotherprop: int = lt.property(default=whatever, readonly=True)

because in both cases, the code that generates the Thing Description and the endpoints is looking at the readonly attribute of the descriptor.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In test_property.py you can search for ro_functional_property_with_setter or localonlysetting, both of which use this form of read-only.

I realise having test_property and test_properties is confusing. test_property is the more recently written test module, which is rather more bottom-up than the older module.

@rwb27 rwb27 added this to the v0.2.0 milestone Mar 3, 2026
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

Successfully merging this pull request may close these issues.

Reset to default feature for properties/settings

2 participants