Skip to content

Raise with a message when reading a value that has no value and no reason - #134

Merged
jwrosewell merged 2 commits into
rcfrom
fix/no-value-message
Nov 15, 2026
Merged

jwrosewell merged 2 commits into
rcfrom
fix/no-value-message

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Nov 14, 2026 •

Copy link
Copy Markdown
Contributor

Reading value() of an AspectPropertyValue that has no value and no reason raised Exception(None), whose message is "None", and an empty reason gave an empty message. The specification says the exception explains why the value is not set, so it now raises with "This instance does not have a set value" in those cases, the message .NET and PHP use.

What the specification says

properties.md, Null values, at 6570727d:

Where 'no value' has been assigned, attempting to access the value MUST throw an error/exception with a customizable message explaining why the value is not set.

.NET's AspectPropertyValue defaults its NoValueMessage to "This instance does not have a set value" (AspectPropertyValue.cs lines 47 and 48 at bb1b5c7a).

What was found

At rc cf904f8, aspectproperty_value.py line 80 raised Exception(self.no_value_message()), so AspectPropertyValue() raised an exception whose message is "None". The specification's "customizable" part was already there, as a reason given to the constructor is the message.

What changed

Files are linked at the head, c7e8e960.

aspectproperty_value.py gains the module constant NO_VALUE_MESSAGE, and value() raises with no_value_message() or, when that is None or empty, with NO_VALUE_MESSAGE. no_value_message() is unchanged and still gives None when no reason was given, so the JSON builder still writes "Unknown" as the null reason in that case (jsonbundler.py lines 88 and 89).

The test, pipeline/fiftyone_pipeline_core/tests/test_aspectproperty_value.py, reads value() with no reason, a None reason, an empty reason and a reason, expecting each to raise with the right message, checks that no_value_message() stays None with no reason, and reads back each value that is set, including None, False, 0, the empty string and a list.

Runs

Each throwaway run gives every package a version as ci/write-package-versions.ps1 does and runs the core package's tox environment, on Ubuntu with Python 3.10 and 3.14.

  • Red, run 43590107707, failure. Its tree is the test commit d27552f on rc cf904f8 and the throwaway workflow. With both versions "3 failed, 484 passed", the three being no reason and a None reason, which raised with "None", and an empty reason, which raised with "".
  • Green, run 43590115300, success. Its tree is the head c7e8e96 and the throwaway workflow. With both versions "484 passed".
  • Full build, run 43590775623 from tmp/pr-matrix-134, success on its second attempt. Configure, all ten Build and Test jobs (Ubuntu and Windows, Python 3.10 to 3.14) and Complete are green. All 11 Setting 'PullRequestSha' lines name the head c7e8e960, and Configure printed "Left out 5 macOS entries". The first attempt failed only in the Ubuntu Python 3.13 job, where the shared Selenium contract test Example_RendersClientSideOverridesOnThePage could not start its browser driver ("Driver service process exited unexpectedly before initialization completed"), and that job alone was run again. That fault is in the shared Selenium contract tests, which selenium-api-tests#28 addresses by starting each browser once, with a longer limit, before the first test that uses it, and selenium-api-tests#31 tracks the 60 second limit every later session keeps.
  • Full build again, run 43666583983 from tmp/pr-matrix-134c, on the same head against rc at cf520fe, which holds this week's other changes, success on its first attempt. All ten Build and Test jobs (Ubuntu and Windows, Python 3.10 to 3.14) and Complete are green, test_aspectproperty_value.py passes in each, all 11 Setting 'PullRequestSha' lines name c7e8e96, and Configure printed "Left out 5 macOS entries".

macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.

Before you upgrade

Code that catches the exception value() raises with no value and reads its message gets "This instance does not have a set value" where it got "None" before, when no reason was given.

What remains

  • AspectPropertyValue still treats the string "noValue" passed as the value as no value, because the constructor uses it as the default for "no value given". Changing that changes the constructor, and is not done here. #155 tracks it.

…message

The Null values section of properties.md says reading a value that is
not set throws with a message explaining why. With no reason given the
value raised Exception(None), whose message is 'None'.
…e and no reason

value() raised Exception(None) when no reason was given. It now raises
with NO_VALUE_MESSAGE, This instance does not have a set value, the
message .NET and PHP use. no_value_message() is unchanged, so the JSON
builder still writes Unknown as the null reason in that case.
@jwrosewell
jwrosewell marked this pull request as ready for review November 15, 2026 04:02
@jwrosewell
jwrosewell merged commit ceb27f9 into rc Nov 15, 2026
3 checks passed
@jwrosewell
jwrosewell deleted the fix/no-value-message branch November 15, 2026 04:02
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.

1 participant