-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow CompactAxis in check_axis #9
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point!
This wasn't handled because the "Compact" expression is not very compact in this case, so we never use it. But it is indeed allowed.
I added two comments. Can you also make the same change for this case?
# Check for single value of allowed axes
It would be great if you could also add a unit test. A copy of the existing spec-domain-point.json
test that uses the compact form would work fine. It would test both the "required" and "allowed" case.
Test case added. |
Co-authored-by: Lukas Phaf <[email protected]>
Co-authored-by: Lukas Phaf <[email protected]>
Co-authored-by: Paul van Schayck <[email protected]>
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #9 +/- ##
==========================================
- Coverage 94.54% 93.90% -0.64%
==========================================
Files 9 9
Lines 275 279 +4
==========================================
+ Hits 260 262 +2
- Misses 15 17 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution!
As far as I understand https://docs.ogc.org/cs/21-069r2/21-069r2.html#_d5c16418-1a20-4dbf-bf7a-8e685062df97 both ValueAxis and CompactAxis should be allowed. The function check_axis currently only allow one of them. This pull request expands to allow both.