Skip to content

Geowave 385#390

Merged
chrisbennight merged 3 commits intomasterfrom
GEOWAVE-385
May 20, 2015
Merged

Geowave 385#390
chrisbennight merged 3 commits intomasterfrom
GEOWAVE-385

Conversation

@rwgdrummer
Copy link
Copy Markdown
Contributor

No description provided.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.01%) to 62.61% when pulling 13409f6 on GEOWAVE-385 into 3785f05 on master.

1 similar comment
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.01%) to 62.61% when pulling 13409f6 on GEOWAVE-385 into 3785f05 on master.

@rwgdrummer
Copy link
Copy Markdown
Contributor Author

FYI. Walked through this with Rich. Had an extra 'compensation' logic for adapting different versions of feature data adapter stored images. Took the adaptive component out by left the version byte in there for future use.

Just to recap:
CRS:84 does not have an SRID...so it fails in several unit tests.
urn:x-ogc:def:crs:EPSG:4326 forces long/lat, as does the current method of passing 'true' to the CRS.decode method with code=EPSG:4326.
EPSG:4326, in itself, cannot force lat/long. It all depends on the authority within the system. For us our purposes, this is ok.

The intent of this fix is to retain the axis ordering provided at the time of construction of a feature writable and feature data type. More critically, if the order is provided in long/lat, then that ordering is preserved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we really want to return an empty string if we can't get a CoordinateSystem?
If we don't want to deal with error propagation back for a nonsense case them maybe use a guava precondition instead?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do to the complexity of some of the CRS managed in geotools, I am not sure that we guarantee that the absence of an axis is an erroneous condition. I interpret such cases as undefined or assumed. Although I could test a hand full of CRS. If they all define an axis, then we could go the route of the error. Or we could just accept the undefined case. Then, the question is whether an formal exception is necessary in these cases, a defined return value or , in this case, an innocuous return value.

Thoughts?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking it over a bit and having a quick discussion with Rich, decided to stick with our default, EPSG:4326 with Axis EAST.

@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.03%) to 62.62% when pulling 5f5440c on GEOWAVE-385 into 9745bb4 on master.

chrisbennight added a commit that referenced this pull request May 20, 2015
@chrisbennight chrisbennight merged commit 440257a into master May 20, 2015
@chrisbennight chrisbennight deleted the GEOWAVE-385 branch June 27, 2015 15:16
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.

3 participants