Skip to content

Commit 506b0af

Browse files
committed
Integrate validity check to update_config
1 parent 618125d commit 506b0af

File tree

7 files changed

+129
-32
lines changed

7 files changed

+129
-32
lines changed

libs/labelbox/src/labelbox/schema/workflow/__init__.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@
4444
# Import from monolithic project_filter.py file
4545
from labelbox.schema.workflow.project_filter import (
4646
ProjectWorkflowFilter,
47-
created_by,
4847
labeled_by,
48+
created_by, # Deprecated, use labeled_by instead
4949
annotation,
5050
dataset,
5151
issue_category,
@@ -95,8 +95,8 @@
9595
"ProjectWorkflowGraph",
9696
"ProjectWorkflowFilter",
9797
# Filter construction functions
98-
"created_by",
9998
"labeled_by",
99+
"created_by", # Deprecated, use labeled_by instead
100100
"annotation",
101101
"sample",
102102
"dataset",

libs/labelbox/src/labelbox/schema/workflow/enums.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ class FilterField(str, Enum):
106106
"""
107107

108108
# User and creation filters
109-
CreatedBy = "CreatedBy"
109+
LabeledBy = "CreatedBy" # Maps to backend CreatedBy field
110110

111111
# Annotation and content filters
112112
Annotation = "Annotation"

libs/labelbox/src/labelbox/schema/workflow/nodes/logic_node.py

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ def remove_filter(self, filter_field: FilterField) -> "LogicNode":
201201
202202
Args:
203203
filter_field: FilterField enum value specifying which filter type to remove
204-
(e.g., FilterField.CreatedBy, FilterField.Sample, FilterField.LabelingTime)
204+
(e.g., FilterField.LabeledBy, FilterField.Sample, FilterField.LabelingTime)
205205
206206
Returns:
207207
LogicNode: Self for method chaining
@@ -211,7 +211,7 @@ def remove_filter(self, filter_field: FilterField) -> "LogicNode":
211211
>>>
212212
>>> # Type-safe enum approach (required)
213213
>>> logic.remove_filter(FilterField.Sample)
214-
>>> logic.remove_filter(FilterField.CreatedBy)
214+
>>> logic.remove_filter(FilterField.LabeledBy) # Consistent with labeled_by() function
215215
>>> logic.remove_filter(FilterField.LabelingTime)
216216
>>> logic.remove_filter(FilterField.Metadata)
217217
"""
@@ -373,7 +373,7 @@ def get_filters(self) -> "ProjectWorkflowFilter":
373373
>>> logic = workflow.get_node_by_id("some-logic-node-id")
374374
>>> user_filters = logic.get_filters()
375375
>>> # Add a new filter
376-
>>> user_filters.append(created_by(["new-user-id"]))
376+
>>> user_filters.append(labeled_by(["new-user-id"]))
377377
>>> # Apply the updated filters back to the node
378378
>>> logic.set_filters(user_filters)
379379
"""
@@ -395,25 +395,25 @@ def add_filter(self, filter_rule: Dict[str, Any]) -> "LogicNode":
395395
396396
Args:
397397
filter_rule: Filter rule from filter functions
398-
(e.g., created_by(["user_id"]), labeling_time.greater_than(300))
398+
(e.g., labeled_by(["user_id"]), labeling_time.greater_than(300))
399399
400400
Returns:
401401
LogicNode: Self for method chaining
402402
403403
Example:
404-
>>> from labelbox.schema.workflow.project_filter import created_by, labeling_time, metadata, condition
404+
>>> from labelbox.schema.workflow.project_filter import labeled_by, labeling_time, metadata, condition
405405
>>>
406-
>>> logic.add_filter(created_by(["user-123"]))
406+
>>> logic.add_filter(labeled_by(["user-123"]))
407407
>>> logic.add_filter(labeling_time.greater_than(300))
408408
>>> logic.add_filter(metadata([condition.contains("tag", "test")]))
409-
>>> # Adding another created_by filter will replace the previous one
410-
>>> logic.add_filter(created_by(["user-456"])) # Replaces previous created_by filter
409+
>>> # Adding another labeled_by filter will replace the previous one
410+
>>> logic.add_filter(labeled_by(["user-456"])) # Replaces previous labeled_by filter
411411
"""
412412
# Validate that this looks like filter function output
413413
if not self._is_filter_function_output(filter_rule):
414414
raise ValueError(
415415
"add_filter() only accepts output from filter functions. "
416-
"Use functions like created_by(), labeling_time.greater_than(), etc."
416+
"Use functions like labeled_by(), labeling_time.greater_than(), etc."
417417
)
418418

419419
# Get the field name from the filter rule to check for existing filters
@@ -457,7 +457,7 @@ def _is_filter_function_output(self, filter_rule: Dict[str, Any]) -> bool:
457457

458458
# Map backend field names to FilterField enum values
459459
backend_to_field = {
460-
"CreatedBy": FilterField.CreatedBy,
460+
"CreatedBy": FilterField.LabeledBy, # Backend CreatedBy maps to user-facing LabeledBy
461461
"Annotation": FilterField.Annotation,
462462
"LabeledAt": FilterField.LabeledAt,
463463
"Sample": FilterField.Sample,

libs/labelbox/src/labelbox/schema/workflow/project_filter.py

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -302,10 +302,10 @@ def metadata(
302302
return result
303303

304304

305-
def created_by(
305+
def labeled_by(
306306
user_ids: List[str], label: Optional[str] = None
307307
) -> Dict[str, Any]:
308-
"""Filter by users who created the labels.
308+
"""Filter by users who labeled the data.
309309
310310
Args:
311311
user_ids: List of user IDs
@@ -320,10 +320,13 @@ def created_by(
320320
return result
321321

322322

323-
def labeled_by(
323+
def created_by(
324324
user_ids: List[str], label: Optional[str] = None
325325
) -> Dict[str, Any]:
326-
"""Filter by users who labeled the data.
326+
"""Filter by users who created the labels.
327+
328+
.. deprecated:: 2.1.0
329+
Use `labeled_by()` instead. This function will be removed in a future version.
327330
328331
Args:
329332
user_ids: List of user IDs
@@ -332,10 +335,15 @@ def labeled_by(
332335
Returns:
333336
Dict representing the filter rule
334337
"""
335-
result: Dict[str, Any] = {"LabeledBy": user_ids}
336-
if label is not None:
337-
result["__label"] = label
338-
return result
338+
import warnings
339+
340+
warnings.warn(
341+
"created_by() is deprecated and will be removed in a future version. "
342+
"Use labeled_by() instead.",
343+
DeprecationWarning,
344+
stacklevel=2,
345+
)
346+
return labeled_by(user_ids, label)
339347

340348

341349
def annotation(
@@ -575,7 +583,7 @@ class ProjectWorkflowFilter(BaseModel):
575583
576584
Example Usage:
577585
filters = ProjectWorkflowFilter([
578-
created_by(["user-123"]),
586+
labeled_by(["user-123"]),
579587
sample(20),
580588
labeled_at.between("2024-01-01", "2024-12-31"),
581589
metadata([condition.contains("tag", "test")]),
@@ -586,7 +594,7 @@ class ProjectWorkflowFilter(BaseModel):
586594
logic.set_filters(filters)
587595
588596
# Or add individual filters
589-
logic.add_filter(created_by(["user-123"]))
597+
logic.add_filter(labeled_by(["user-123"]))
590598
"""
591599

592600
rules: List[Dict[str, Any]] = Field(default_factory=lambda: [])
@@ -636,7 +644,7 @@ def _validate_filter_structure(self, rule: Dict[str, Any]) -> None:
636644
if not isinstance(rule, dict) or not rule:
637645
raise ValueError(
638646
"Filters must be created using filter functions. "
639-
"Use functions like created_by([...]), metadata([...]), labeled_at.between(...), etc."
647+
"Use functions like labeled_by([...]), metadata([...]), labeled_at.between(...), etc."
640648
)
641649

642650
# Basic structural validation - ensure we have at least one field

libs/labelbox/src/labelbox/schema/workflow/workflow.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,16 +390,36 @@ def validate(cls, workflow: "ProjectWorkflow") -> "ProjectWorkflow":
390390
def update_config(self, reposition: bool = True) -> "ProjectWorkflow":
391391
"""Update the workflow configuration on the server.
392392
393+
This method automatically validates the workflow before updating to ensure
394+
data integrity and prevent invalid configurations from being saved.
395+
393396
Args:
394397
reposition: Whether to automatically reposition nodes before update
395398
396399
Returns:
397400
ProjectWorkflow: Updated workflow instance
398401
399402
Raises:
400-
ValueError: If the update operation fails
403+
ValueError: If validation errors are found or the update operation fails
401404
"""
402405
try:
406+
# Always validate workflow before updating (mandatory for data safety)
407+
validation_result = self.check_validity()
408+
validation_errors = validation_result.get("errors", [])
409+
410+
if validation_errors:
411+
# Format validation errors for clear user feedback
412+
formatted_errors = self.format_validation_errors(
413+
validation_result
414+
)
415+
logger.error(f"Workflow validation failed: {formatted_errors}")
416+
417+
# Raise a clear ValueError with validation details
418+
raise ValueError(
419+
f"Cannot update workflow configuration due to validation errors:\n{formatted_errors}\n\n"
420+
f"Please fix these issues before updating."
421+
)
422+
403423
if reposition:
404424
self.reposition_nodes()
405425

libs/labelbox/src/labelbox/schema/workflow/workflow_utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ def validate(cls, workflow: "ProjectWorkflow") -> "ProjectWorkflow":
180180
errors.extend(connection_errors)
181181

182182
# Store validation results
183-
workflow._validation_errors = {"validation": errors}
183+
workflow._validation_errors = {"errors": errors}
184184
return workflow
185185

186186
@staticmethod

libs/labelbox/tests/integration/test_workflow.py

Lines changed: 75 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@
2121
WorkflowDefinitionId,
2222
FilterField,
2323
# Import filter functions
24-
created_by,
24+
labeled_by,
25+
created_by, # Still works for backward compatibility
2526
dataset,
2627
natural_language,
2728
labeling_time,
@@ -335,6 +336,72 @@ def test_workflow_update_without_reset(client, test_projects):
335336
assert review_nodes[0].name == "Updated Review"
336337

337338

339+
def test_workflow_validation_in_update_config(client, test_projects):
340+
"""Test the mandatory validation behavior in update_config."""
341+
source_project, _ = test_projects
342+
343+
# Create an invalid workflow (missing connections)
344+
workflow = source_project.get_workflow()
345+
workflow.reset_config()
346+
347+
# Add nodes but don't create proper connections (this will be invalid)
348+
initial_labeling = workflow.add_node(type=NodeType.InitialLabeling)
349+
initial_rework = workflow.add_node(type=NodeType.InitialRework)
350+
# Add a review node with no connections - this should cause validation errors
351+
review = workflow.add_node(type=NodeType.Review, name="Unconnected Review")
352+
353+
# Only connect the initial nodes together, leaving review disconnected
354+
workflow.add_edge(initial_labeling, initial_rework) # This is also invalid
355+
356+
# Test 1: update_config should validate and fail with invalid workflow
357+
with pytest.raises(ValueError) as exc_info:
358+
workflow.update_config()
359+
360+
assert "validation errors" in str(exc_info.value).lower()
361+
assert "Cannot update workflow configuration" in str(exc_info.value)
362+
363+
# Test 2: Multiple calls should consistently fail validation
364+
with pytest.raises(ValueError) as exc_info:
365+
workflow.update_config()
366+
367+
assert "validation errors" in str(exc_info.value).lower()
368+
369+
# Test 3: Validation errors should be consistently reported
370+
with pytest.raises(ValueError) as exc_info:
371+
workflow.update_config()
372+
373+
# Verify the error message is clear and helpful
374+
error_message = str(exc_info.value)
375+
assert "validation errors" in error_message.lower()
376+
assert "please fix these issues" in error_message.lower()
377+
378+
# Test 4: Create a valid workflow and test successful update
379+
workflow.reset_config()
380+
381+
initial_labeling = workflow.add_node(type=NodeType.InitialLabeling)
382+
initial_rework = workflow.add_node(type=NodeType.InitialRework)
383+
review = workflow.add_node(type=NodeType.Review, name="Connected Review")
384+
done = workflow.add_node(type=NodeType.Done, name="Final")
385+
386+
# Create proper connections
387+
workflow.add_edge(initial_labeling, review)
388+
workflow.add_edge(initial_rework, review)
389+
workflow.add_edge(review, done, NodeOutput.Approved)
390+
391+
# This should work without errors
392+
result = workflow.update_config()
393+
assert result is not None
394+
395+
# Test successful update - should not raise any exceptions
396+
try:
397+
workflow.update_config()
398+
# If we get here, the update was successful
399+
assert True
400+
except ValueError:
401+
# Should not happen with a valid workflow
402+
assert False, "Valid workflow should not raise validation errors"
403+
404+
338405
def test_workflow_copy(client, test_projects):
339406
"""Test copying a workflow between projects."""
340407
source_project, target_project = test_projects
@@ -353,7 +420,7 @@ def test_workflow_copy(client, test_projects):
353420
logic = source_workflow.add_node(
354421
type=NodeType.Logic,
355422
name="Source Logic",
356-
filters=ProjectWorkflowFilter([created_by(["source-user"])]),
423+
filters=ProjectWorkflowFilter([labeled_by(["source-user"])]),
357424
)
358425
done = source_workflow.add_node(type=NodeType.Done, name="Source Done")
359426

@@ -402,7 +469,7 @@ def test_production_logic_node_with_comprehensive_filters(
402469
match_filters=MatchFilters.Any,
403470
filters=ProjectWorkflowFilter(
404471
[
405-
created_by(
472+
labeled_by(
406473
["cly7gzohg07zz07v5fqs63zmx", "cl7k7a9x1764808vk6bm1hf8e"]
407474
),
408475
metadata([m_condition.contains("tag", ["test"])]),
@@ -491,7 +558,9 @@ def test_filter_operations_with_persistence(client, test_projects):
491558
name="Filter Test",
492559
filters=ProjectWorkflowFilter(
493560
[
494-
created_by(["user1", "user2"]),
561+
created_by(
562+
["user1", "user2"]
563+
), # Still works - backward compatibility
495564
sample(30),
496565
labeling_time.greater_than(500),
497566
]
@@ -518,7 +587,7 @@ def test_filter_operations_with_persistence(client, test_projects):
518587
), f"Should start with 3 filters, got {initial_count}"
519588

520589
# Test removing filters with persistence
521-
logic_node.remove_filter(FilterField.CreatedBy)
590+
logic_node.remove_filter(FilterField.LabeledBy)
522591
logic_node.remove_filter(FilterField.Sample)
523592
updated_workflow.update_config(reposition=False)
524593

@@ -542,7 +611,7 @@ def test_filter_operations_with_persistence(client, test_projects):
542611
), "LabelingTime filter should remain"
543612
assert (
544613
"CreatedBy" not in remaining_fields
545-
), "CreatedBy filter should be removed"
614+
), "LabeledBy filter should be removed"
546615

547616
# Test adding filters with persistence
548617
logic_after_removal.add_filter(dataset(["new-dataset"]))

0 commit comments

Comments
 (0)