Skip to content

Conversation

kleintom
Copy link
Contributor

@kleintom kleintom commented Jun 6, 2025

Ready for review but this may need some discussion, so marking as draft for now.
Changes:

  • Left and right annotators offer the same options
  • The options have changed from combined-create-update (left) and create (right) to Add, Remove, Replace - the same options offered for Confidences and Protocols.
  • Operations are now using batch_by_filter_scope

@kleintom
Copy link
Contributor Author

kleintom commented Jun 6, 2025

Notes on BatchResponse return counts from batch_by_filter_scope.

  1. I've tried here to return values for all operations, and to spec those returns. One subtle point: if you're mass-annotating otu DAs, e.g.
  • if you're doing an Add, then the total_attempted count returned will be the number of otus in your filter, and the updated count will be the number you actually added a DA to
  • OTOH, if you're doing a Replace, then the total_attempted will be the total number of DAs on your otus (possibly larger than the number of otus) and the updated will be the number of those you actually updated
  1. At least in the cases I've worked on so far, it's seeming like just returning a count has been more natural than returning a list of ids. Particularly for remove, when the ids you might return would reference data that has been deleted, but also in the case of mass updates where gathering ids would be extra work compared to just a count. Currently this leads to the pattern updated = Array.new(count). Maybe consider a change to BatchResponse?

@kleintom kleintom marked this pull request as draft June 6, 2025 14:01
@mjy
Copy link
Member

mjy commented Jun 6, 2025

@kleintom can you screenshot the UI now if you get a chance?

@kleintom
Copy link
Contributor Author

kleintom commented Jun 6, 2025

image
image

On replace you can only replaces value, not predicate and value.

@mjy
Copy link
Member

mjy commented Aug 26, 2025

Add, replace, remove as chainable operations seems clean. Putting this on pause until we get more feedback from users. Discussed with @kleintom

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.

2 participants