Skip to content
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

Should locations be compared as Strings? #35

Open
albertopq opened this issue Jan 24, 2017 · 7 comments
Open

Should locations be compared as Strings? #35

albertopq opened this issue Jan 24, 2017 · 7 comments
Assignees

Comments

@albertopq
Copy link

I'm running into an issue when ordering by Locations.location.
I think that should compare results by coordinates, but the test is comparing as a String, according to:

https://github.com/opengeospatial/ets-sta10/blob/master/src/main/java/org/opengis/cite/sta10/filteringExtension/Capability3Tests.java#L537

We are delegating that orderby to SQL (postgres) directly and it seems that the result is different:

coordinates: [-117.05, 51.05] < coordinates: [-100.05,50.05]

when the test is expecting the opposite.

Could somebody please elaborate on the idea behind this test?
Thanks!

@hylkevds
Copy link
Contributor

hylkevds commented Jan 24, 2017

It should probably exclude Locations.location from the orderby tests, since ordering by location does not make sense. Especially when considering polygon locations.
Currently there is only an exclusion of unitOfMeasurement. Thing.properties, Observation.parameters and FeatureOfInterest.feature should probably also be excluded from the sorting tests.

@taniakhalafbeigi
Copy link
Contributor

Thanks for pointing that out.
And yes, I agree. We'd better exclude all Thing.properties, Observation.parameters, FeatureOfInterest.feature, and Location.location from $orderby test.

@hylkevds
Copy link
Contributor

hylkevds commented Jan 24, 2017

I've fixed this in the last commit on the pull request #32 since it nicely fits in that set. Instead of just using a list of string, it introduces a class EntityProperty that has several fields that are useful for the checks.
The commit also fixed several other places where properties where not tested quite right.

If you feel adventurous you could test our fork of the test suit: https://github.com/FraunhoferIOSB/ets-sta10

@albertopq
Copy link
Author

That sounds good. What are the next steps then? When is the PR expected to land? Should I submit a smaller PR that only fixes that?

Thanks!

@mariakrommyda
Copy link

Hello everyone,

According to the discussion above the unitOfMeasurement, Thing.properties, Observation.parameters, FeatureOfInterest.feature, and Location.location should be excluded from the order by test.
In line 320-322 though of the relevant file, ets-sta10/src/main/java/org/opengis/cite/sta10/filteringExtension/Capability3Tests.java, only the unitOfMeasurement is excluded.
Am I missing something or this is a still pending correction?

Best regards,
Maria.

@dstenger
Copy link
Contributor

dstenger commented May 2, 2019

@taniakhalafbeigi Can you please take a look at this question? What is the current status of the test suite here?

@taniakhalafbeigi
Copy link
Contributor

Currently there is no PR specific to this issue and this issue hasn't been addressed yet.

@dstenger dstenger added this to CITE Aug 1, 2024
@dstenger dstenger moved this to To do in CITE Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: To do
Development

No branches or pull requests

5 participants