Add xl_chart() and xl_chart_series() constructors - #136
Merged
Conversation
First commit of Phase 11a. Objects, validation and the type-feature matrix;
nothing reaches a file until apply_charts() lands.
Placement reuses xl_image()'s vocabulary exactly -- at, scale, offset,
position, description, decorative -- because lxw_chart_options is
lxw_image_options minus url, tip and cell_format, with the same field names. A
test asserts all six appear in both signatures, so the correspondence cannot
drift silently.
Series ranges carry an optional `sheet` and are left unresolved until the whole
workbook is known, following the table-name precedent: a chart may plot data
from a sheet other than the one it sits on.
The feature matrix exists because libxlsxwriter documents these restrictions in
prose and then ignores them -- chart_set_hole_size() on a pie chart, or up-down
bars on a column chart, are accepted and dropped silently by Excel. Two tests
guard the matrix itself rather than only its contents: every type must map to a
real family, and every feature must be both supported by some type and denied
by some, since a feature nothing supports is dead code and one everything
supports does not belong in a matrix.
Two things the tests forced:
* A series name is always literal text. It cannot be "text or a range": a
series may legitimately be called "Q1!". Taking the name from a cell needs
an explicit spec, which is now documented rather than implied.
* The API-consistency gate fired on xl_hyperlink(format, name) against
xl_chart_series(name, format). This one is a genuine false friend -- in
xl_hyperlink `name` is the deprecated alias for `value`, kept last on
purpose, while here it is the legend label -- so it joins the allow-list
with that reasoning recorded.
Verified before writing any of this, with a pure-C reprex against libxlsxwriter
1.2.4: charts and chartsheets produce a drawing part and so do not desync the
drawing-id counter, they survive constant_memory, and a chart IS a victim of
the desync exactly as a floating image is. That last result is why
.check_drawing_order() will need to count charts, which no existing test would
have caught.
Every exported object carried @family writexl, which made roxygen generate a "See also" section on each page listing all 26 others. A cross-reference that points everywhere points nowhere: the section was noise on every page, and a reader looking for "what else is like this?" got the package index. Six families, by what the reader is trying to do: cell content what goes into a cell -- xl_cell_general, xl_formula, xl_rich_string, xl_rich_run, xl_comment, is_xl_comment cell formatting the format engine -- xl_format, the group constructors, xl_color, is_xl_format worksheet layout geometry, view and printing -- xl_sheet, xl_colrow_spec, xl_sheet_view, xl_outline, xl_page_setup worksheet features things attached to a sheet that act on ranges of its data -- xl_merge, xl_filter, xl_filter_keep, xl_validation, xl_conditional, xl_table, xl_table_column images and charts xl_image, xl_chart, xl_chart_series workbook settings xl_workbook, xl_properties, write_xlsx The grouping fell out cleanly per file -- no file mixes concepts, which is a good sign the source layout already matched the feature boundaries. write_xlsx() gains a family it did not have. It is the entry point, so a reader on xl_workbook() or xl_properties() should be led back to it; that link did not exist before. xl_outline sits under layout rather than features because it draws the grouping controls that xl_col_spec(level =) creates -- it is about display, not about the data.
Brings in the constant_memory decision procedure (#135). One conflict, in R/write_xlsx.R: both sides inserted at the same point in the roxygen block, master adding the constant_memory and constant_memory_threshold @PARAM entries and this branch adding @family workbook settings. Both are wanted, so both are kept, with the params before the family tag as in every other file. Checked rather than assumed: all 28 @family tags survive with their feature groupings, none reverted to "writexl", and the merged files still carry the constant_memory and allow_sheet work from master.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First commit of Phase 11a. Objects, validation and the type-feature matrix; nothing reaches a file until apply_charts() lands.
Placement reuses xl_image()'s vocabulary exactly -- at, scale, offset, position, description, decorative -- because lxw_chart_options is lxw_image_options minus url, tip and cell_format, with the same field names. A test asserts all six appear in both signatures, so the correspondence cannot drift silently.
Series ranges carry an optional
sheetand are left unresolved until the whole workbook is known, following the table-name precedent: a chart may plot data from a sheet other than the one it sits on.The feature matrix exists because libxlsxwriter documents these restrictions in prose and then ignores them -- chart_set_hole_size() on a pie chart, or up-down bars on a column chart, are accepted and dropped silently by Excel. Two tests guard the matrix itself rather than only its contents: every type must map to a real family, and every feature must be both supported by some type and denied by some, since a feature nothing supports is dead code and one everything supports does not belong in a matrix.
Two things the tests forced:
A series name is always literal text. It cannot be "text or a range": a series may legitimately be called "Q1!". Taking the name from a cell needs an explicit spec, which is now documented rather than implied.
The API-consistency gate fired on xl_hyperlink(format, name) against xl_chart_series(name, format). This one is a genuine false friend -- in xl_hyperlink
nameis the deprecated alias forvalue, kept last on purpose, while here it is the legend label -- so it joins the allow-list with that reasoning recorded.Verified before writing any of this, with a pure-C reprex against libxlsxwriter 1.2.4: charts and chartsheets produce a drawing part and so do not desync the drawing-id counter, they survive constant_memory, and a chart IS a victim of the desync exactly as a floating image is. That last result is why .check_drawing_order() will need to count charts, which no existing test would have caught.