Ebuttd/Unnesting divs, spans #38
Conversation
…t directory for denester
…identical styles) Adapting existing tests to work with functions that have been modified Modifying functions used for unnesting spans, correcting the order of inheritance for styles Modifying the functions that calculated text size to work with more than two styles check-equal method added to the style_type class Updated expected xml used in tests to more accurately reflect expected output
Added more tests for nesting styles
PhoebeWheelerCode
force-pushed
the
ebuttd/MSPA-458-nested-elements
branch
from
fd60e31 to
09f2591
Compare
Handles a bug where a p with no explicit start time will take the earliest computed begin time of a span it contains, causing spans to calculate their begin/end times incorrectly
There was a problem hiding this comment.
I'm pushing review comments now even though there's another issue that isn't directly commented on: some of the timing tests fail. I'm mid-way through trying to trace through because they are complex edge cases and probably not the fault of any code here. I'll push some fixes to that separately.
… of milliseconds regardless of number of digits Previously, `00:00:00.17` was converted to a timedelta with _17_ milliseconds instead of 170, and resulted in a different value from `00:00:00.170`, but the spec says stuff after the decimal point is a decimal fraction not a number of milliseconds. This commit truncates any digits in the fraction beyond the third, without rounding.
Catch regressions to any timedelta conversions from timing types
Removing unnecessary print lines Adding comments to explain complex functions Using timedelta to make certain functions unnecessary so they can be removed Replacing strings in some tests with FullClockTimingType to better represent actual input that would be received
Rename TestNester to TestDenester
|
I've reviewed the changes up to now, all looking good, thanks @PhoebeWheelerCode - I understand you're looking at the last review comment too so will review that change when you've done it. |
Modifying tests to work with the DenesterNode Removing some facet-based code in tests that was missed when facets were removed
…unt for this Removed creation of DenesterNode inside converters.py Modified tests to account for changes in Denester indentation
|
Still have rst file to do but have pushed fixes for other recent minor comments |
Added an rst file for the Denester node
This could do with a standard config file and an `ebu-run` command line to run it to demonstrate it in action.
There was a problem hiding this comment.
It is possible (even likely) that with a change this complex some bugs are present, but it's good enough that we should merge it now and raise any issues we find later to solve them another time. It's really complex, so overall great work - I am not trying to do it down! Thanks @PhoebeWheelerCode and @mm326 for your hard work on this (and sorry if I've missed anyone out!)
Code for unnesting various nested elements, correctly inheriting/combining different aspects of them (languages, regions, font sizes, etc)
The divs are recursively iterated through until something other than a div (such as a 'p') is found. A new div is created to contain the non-div, using the merged attributes of the parent divs, and these new divs are added to a list. These then replace the old divs.
The new unnested spans are created using a similar function, (iterating until they find something other than a span) with different code for combining their attributes. It also has functions for calculating the font size and correctly inheriting attributes/combining styles.
There are measures in place to prevent the code from creating styles that are functionally identical to an existing one (except in name) to prevent duplicates and keep the overall number of styles down.
It also does not create a new style when it would have the name of an existing style. This may pose an issue if two different style combinations end up producing the same name, eg:
These combined styles later have their font sizes removed and made into their own styles, meaning that duplicate styles (identical in content but not in name) are possible in the end result.