660: nicer error about choices sheet when extension omitted in select_.._from_file #673
Conversation
This file contains 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
…rm#602) - found this issue because `error__contains` is the correct spelling, so the message was not being checked (the pyxformtestcase kwarg validation does not occur if the form has errors). - the error for the entities sheet should report the problematic columns in the order they appear. Until XLSForm#602 is merged, the incoming dict may have random key order. This change avoids re-introducing the same sort of issue via set usage, instead doing the same with dict keys. - to avoid tests randomly failing, test `error__contains` looks for each column individually.
- previously, an error about a missing sheet name may be thrown, e.g. if the form didn't contain any regular choices. This change explicitly checks for file extension, similar to external selects. - added test case for all the combinations, and removed duplicative older tests - added new alias subgroup for select..from_file to reduce duplication
lognaturel
approved these changes
Dec 1, 2023
|
Yay, @lindsay-stevens out here making my life so much better! (And hopefully that of other users, too 😊) |
|
Just got this error again, such a big improvement! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes #660
Why is this the best possible solution? Were any other approaches considered?
Previously, an error about a missing sheet name may be thrown, e.g. if the form didn't contain any regular choices. This change explicitly checks for file extension, in order to show a nicer error message. The existing constants, aliases, and validation pattern are used.
What are the regression risks?
I think this is just an enhancement, since the old behaviour was confusing, and in both cases an error is raised.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No it's already documented in the template, just an easy mistake to make
Before submitting this PR, please make sure you have:
testsnosetestsand verified all tests passblack pyxform teststo format code