Repository navigation
Feature/ppa page alignment - #285
Conversation
Assisted-by: Zed:claude-sonnet-4-6
Assisted-by: Zed:claude-sonnet-4-6
Assisted-by: ClaudeCode:claude-sonnet-4-6
Assisted-by: Zed:claude-sonnet-4-6
Assisted-by: Zed:claude-sonnet-4-6
per review from ClaudeCode:claude-sonnet-4-6
Assisted-by: ClaudeCode:claude-opus-4-7
Assisted-by: ClaudeCode:claude-opus-4.7
Assisted-by: ClaudeCode:claude-opus-4.7
Assisted-by: ClaudeCode:claude-opus-4.7
Assisted-by: ClaudeCode:claude-opus-4.7
Assisted-by: OpenCode:claude-opus-4.8
Assisted-by: OpenCode:claude-opus-4.8
Assisted-by: OpenCode:claude-opus-4.8
Assisted-by: OpenCode:claude-opus-4.8
Assisted-by: OpenCode:claude-opus-4.8
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #285 +/- ##
===========================================
+ Coverage 82.18% 83.63% +1.44%
===========================================
Files 23 24 +1
Lines 2156 2468 +312
===========================================
+ Hits 1772 2064 +292
- Misses 384 404 +20 🚀 New features to boost your workflow:
|
Assisted-by: OpenCode:claude-opus-4.8
Assisted-by: OpenCode:claude-opus-4.8
laurejt
left a comment
There was a problem hiding this comment.
I reviewed dataset_prep.py. It seemed good overall, but I think it might be changing the shift logic a bit so that the shift value is more intuitive (and would correspond to the appropriate shift action in polars).
Note that most of the comments will be misaligned due to file changes while drafting my review.
| a single vectorized fuzzy comparison; short pages inherit the shift of the | ||
| surrounding long pages. Contiguous runs of equal shift are handled |
There was a problem hiding this comment.
If the two long pages shift in different ways, how does inheritance work?
There was a problem hiding this comment.
Currently the code does a forward fill and then a backward fill - so any short pages at beginning and end of a work or excerpt would get the shift of the nearest long page. Not sure how it works if this happens in the middle but seems like a reasonable solution
There was a problem hiding this comment.
Maybe worth a test case so we know what the expected behavior, but seems reasonable to me.
Co-authored-by: Laure Thompson <602628+laurejt@users.noreply.github.com>
per @laurejt review Assisted-by: OpenCode:claude-opus-4.7
Assisted-by: OpenCode:claude-opus-4.8
Assisted-by: OpenCode:claude-opus-4.8
Assisted-by: OpenCode:claude-opus-4.8
…nment Assisted-by: OpenCode:claude-opus-4.8
Assisted-by: OpenCode:claude-opus-4.8
Assisted-by: OpenCode:claude-opus-4.8
Assisted-by: OpenCode:claude-opus-4.8
Assisted-by: OpenCode:claude-opus-4.8
Assisted-by: OpenCode:claude-opus-4.8
|
I'm going to merge this into develop and consider the script portion done - the page shift logic still needs to be revised, and there may be other small additions (1930 images, old/new ocr), but there are enough changes already that I think merging this set in will be useful. |
Associated Issue(s): resolves #284 #277
Changes in this PR
Notes
align_shifted_pagesandalign_pages- this is the core of the logic for matching up HathiTrust pages with images