Skip to content

763: adding a label for background-geo should not produce body control #858

Open
lindsay-stevens wants to merge 17 commits into
XLSForm:masterfrom
lindsay-stevens:pyxform-763
Open

763: adding a label for background-geo should not produce body control #858
lindsay-stevens wants to merge 17 commits into
XLSForm:masterfrom
lindsay-stevens:pyxform-763

Conversation

@lindsay-stevens

Copy link
Copy Markdown
Contributor

Closes #763

Why is this the best possible solution? Were any other approaches considered?

Branch based on deps-update / PR 853 branch so only commit b6ec9ff is relevant.

Per the commit message:

The cause was that in builder.py, the "trigger" tag maps to the TriggerQuestion class, which calls Question._build_xml(), which creates the control element and label/hint. The alternative "action" tag maps to Question, and the base class Question.build_xml() emits None. Another option is to remove the "control" default completely, which would map background-geo to the base Question class, but it was changed to "action" since a) other adjacent types are doing the same, b) the type is basically an action so it is descriptive. Added label test for each "action" question type.

Considered adding a warning if label is specified, and then started thinking that maybe there should be a warning if any fields that aren't supported by that type are present. Then I started imagining a giant table of all the supported question types in rows and all the supported columns, where each cell is true/false to indicates whether or not that feature works for that type. Something of that nature would be useful documentation for pyxform (and Collect/Webforms) and as a regression testing tool, but it might be a bit too noisy to have pyxform emit a warning for every unsupported row/column combination.

What are the regression risks?

Low risk, maybe someone was relying on the old behaviour but #763 suggests it was not intended and undesirable.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

No (other than as mentioned above).

Before submitting this PR, please make sure you have:

  • included test cases for core behavior and edge cases in tests
  • run python -m unittest and verified all tests pass
  • run ruff format pyxform tests and ruff check pyxform tests to lint code
  • verified that any code or assets from external sources are properly credited in comments

- resolve new warnings on RUF059 (unused vars) and B017 (broad err).
- re-checked commented out rules with ruff version markers, and the
  noted issues are still present, so updated the noted version.
- removes strenum copy which is only present in py3.11+ so that change
  is not backwards compatible. However, ruff warns that keeping it no
  good either since StrEnum exists in py311.
- refurb "A tool for refurbishing and modernizing Python codebases."
- the full pydocstyle ruleset has many changes so enabling some of them
  individually to keep the diffs manageable.
- the full pydocstyle ruleset has many changes so enabling some of them
  individually to keep the diffs manageable.
- the full pydocstyle ruleset has many changes so enabling some of them
  individually to keep the diffs manageable.
- the full pydocstyle ruleset has many changes so enabling some of them
  individually to keep the diffs manageable.
- D415 is a bit strange when it comes to indented inline code blocks,
  so added backticks for those cases as well as URLs.
- the full pydocstyle ruleset has many changes so enabling some of them
  individually to keep the diffs manageable.
- the full pydocstyle ruleset has many changes so enabling some of them
  individually to keep the diffs manageable.
- The previous few commits selectively enabled the pydocstyle rules that
  resulted in changes that seemed reasonable to accept now.
- The rules now in the ignore list are ones that would require a lot of
  manual rewrites (most of them have dozens of hits).
- So this rearrangement enables other potentially useful pydocstyle
  rules that aren't currently triggering changes or warnings.
- `flynt is a command line tool to automatically convert a project's
   Python code from old "%-formatted" and .format(...) strings into
   Python 3.6+'s "f-strings".`
- conversion result in next commit so that file history is kept.
- used `pandoc --wrap=preserve -f rst -t gfm README.rst -o README.md`
  - markdown easier to remember and maintain
- fixed grammatical error L112 "in a easily navigable format"
- removed badges since this info is available on the same page as the
  readme (either GitHub or PyPI) and so one less thing to update.
- The cause was that in builder.py, the "trigger" tag maps to the
  TriggerQuestion class, which calls Question._build_xml(), which
  creates the control element and label/hint. The alternative "action"
  tag maps to Question, and the base class Question.build_xml() emits
  None. Another option is to remove the "control" default completely,
  which would map background-geo to the base Question class, but it
  was changed to "action" since a) other adjacent types are doing the
  same, b) the type is basically an action so it is descriptive.
- added label test for each "action" question type
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.

Specifying a label for a background-geopoint question should not produce a body element

1 participant