Repository navigation
4.6.0 cherrypick recent PRs - #871
Open
lindsay-stevens wants to merge 13 commits into
Open
lindsay-stevens wants to merge 13 commits into
lindsay-stevens wants to merge 13 commits into
Conversation
(cherry picked from commit d414910)
- use isinstance instead of hasattr so it's clearer what object types the code is relevant to (L141, L168, L175). - move "generate_repeating_template" into RepeatingSection since the method is only applicable to this type. (cherry picked from commit 637f780)
- this didn't work anyway, until a couple of commits ago. - there doesn't seem to be any reason for it to be allowed, but it does seem like the sort of thing that might be useful in future for scoping instances or entities so disallowing it now leaves that use case open (as opposed to warning (for no strong reason) and then potentially requiring forms to be updated later). (cherry picked from commit a08469b)
- 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 (cherry picked from commit b6ec9ff)
- this problem is caught by ODK Validate, but the error is somewhat cryptic since it refers to instance() expressions generated by pyxform for entity attributes (as shown in the replaced test test_implicit_update_mode__instance_required__error). - xls2json.py: existing secondary_instances tracking to include the file type, update question_types/geo.py accordingly (cherry picked from commit 6f7ceec)
- previously the code seemed to be repetitively and inconsistently identifying the select types, so now there's 3 distinct branches: external selects, internal selects, select_from_file. (cherry picked from commit 04ab088)
- now suggests adding a list or checking spelling. - it seems there was no existing test for the old message so new tests are added for the pass/fail cases. (cherry picked from commit c1e273b)
- test case includes characters in the broken range as an example. The intent of the regex is to match XML so it should have allowed them. - not quite a regression since although typo has been present since 2024, the previous regex validation only allowed ascii characters. Also the error message just says "letters" are allowed rather than specifying that a particular type of letter is supported. (cherry picked from commit b186bc1)
- previously only lower-cased secondary instances could be used. - as reported on https://forum.getodk.org/t/58592 (cherry picked from commit a895442)
- as described in test case comment, last copy wins (cherry picked from commit 154c221)
(cherry picked from commit 59ef17b)
- although pyproject.toml specifies the flit_core version under the build-system section, pip doesn't read that. The flit project doesn't have an upper bound on flit_core dependency so when a newer version released with a breaking API change, it broke the build.
- consistently use "variable reference ('${name}')" to clarify what
feature these errors are about.
- the variable reference feature resolves any name from the survey
sheet to XPath, including questions/fields or repeat/display groups,
so the terms 'question name', 'field name' are avoided.
- survey 'name' column values are sometimes referred to as 'variables'
and these tokens are "references to variables", but the term
"reference variables" could imply a special type of variable, so
the term "variable reference" is used. The ODK docs refer to this
feature as just "Variables" and so "variable references" separates
this syntax from a survey 'name' column value.
- docs also refer to this feature as '${} notation' but that seemed
like it may be confused with a token substitution error or something
technical; however to further disambiguate the above point, the
example string `('${name}')` is appended to show the syntax and
highlight that the value inside the braces must be a name.
- add include a "Learn more" link to the docs where relevant.
- add error message guideline in ErrorCode docstring.
(cherry picked from commit 08a7784)
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.
Changes from recent PRs, excluding merge commits, applied in chronological order as listed below.
Why is this the best possible solution? Were any other approaches considered?
Included are the lower impact changes/fixes that can be included in a shorter release cycle. More could be added depending on required timing; they should go into
masterfirst then cherry-pick back to a PR branch based onrelease/v4.6.0. Once v4.6.0 is released, the version/changelog commit(s) should be cherry-picked back tomaster.The below unreleased changes on the
master(main) branch are more significant or backwards-incompatible changes, which would go into a v5.x release.What are the regression risks?
Should be minimal, these are mostly bug fixes.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No
Before submitting this PR, please make sure you have:
testspython -m unittestand verified all tests passruff format pyxform testsandruff check pyxform teststo lint code