aws-cqc / aws-cqc/DeviceLayout.jl

Keyword arg slurping leads to silent errors

Open
#7 0 comments 0 reactions 0 assignees View on GitHub
2.0 bug enhancement
Dominant language
Julia
Stars
67
Forks
14
Avg merge
1d 16h
Merged PRs (30d)
21

Description

This is particularly an issue in `render!`, where you can provide rendering options as keyword arguments. The purpose of this is to allow passing user-defined flags such that these eventually make their way down to e.g. `OptionalStyle` and therefore control rendering behavior. The problem with this design is that you can always pass flags that just don't do anything, and an error is never thrown. This can generate output that is wrong/not what you expect and you'd never know.

Proposed solutions:

- Enumerate all built-in rendering keyword options. `max_recursions, max_change, rand_factor, grid_step` get used by `adapted_grid`, while `atol` gets used in a few places (but not `adapted_grid`...). There's also currently `Δθ` in `Ellipse` (which should probably be `atol` again...) and `rounded` for choosing to round the ellipse or not when rendering to `SolidModel` (this one is particularly error-prone—is it rounded, round, rounding?). Then we have `artwork`, `simulation`, and `solidmodel` intended for `OptionalStyle` with different targets. Then we can make a struct and/or `Target` fields that contain exactly these, and force `render!` to take these instead of arbitrary keywords. (At the very least these the built-in options should be spelled out in the `render!` docs.)
- To avoid errors in the other direction (users create their own `OptionalStyle` that uses `round` instead of `rounded`), make these an actual `@enum RenderingOption rounded, atol, ...` and make `OptionalStyle` accept only those. However, users can't extend the enum with their own options.
- We could let users make their own arbitrary options at their own risk, or we could separate built-in options and custom options, but figure out a way to enforce something similar for custom options.
- There are also other things we can do to make options safer. For example, `@info` when an option or default is used, `@warn` when an option is not used (would need to somehow record which options are used during rendering).

Contributor guide

Open the contributing guide

Research direction

Start at render! and trace how keyword arguments reach OptionalStyle, adapted_grid, Ellipse, and SolidModel. Review the listed built-in options and decide which proposal resolves silent unused or misspelled options; done means rendering options are handled consistently and invalid or unused flags cannot silently change nothing.

Written by the indexing model from the issue text.

Assessment

Tech stack
julia
Domain
computer-graphics
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.