Skip to content

added temporal units to support binning by decade, hour, and minute#801

Merged
rwgdrummer merged 1 commit intolocationtech:masterfrom
dcy2003:GEOWAVE-555
May 27, 2016
Merged

added temporal units to support binning by decade, hour, and minute#801
rwgdrummer merged 1 commit intolocationtech:masterfrom
dcy2003:GEOWAVE-555

Conversation

@dcy2003
Copy link
Copy Markdown
Contributor

@dcy2003 dcy2003 commented May 24, 2016

@rfecher
Copy link
Copy Markdown
Contributor

rfecher commented May 24, 2016

To expose it to the commandline tools you have t o add it as options to the periodicity in SpatialTemporalDimensionalityTypeProvider. Also, if you could figure out a way to get "week" in there that would be awesome!

+ TWO_DIGIT_NUMBER.format(value.get(Calendar.MONTH)) + "_"
+ TWO_DIGIT_NUMBER.format(value.get(Calendar.DAY_OF_MONTH)) + "_"
+ TWO_DIGIT_NUMBER.format(value.get(Calendar.HOUR_OF_DAY)) + "_" + TWO_DIGIT_NUMBER
.format(value.get(Calendar.MINUTE))));
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.

May be we should now consider compressing these a bit.

Copy link
Copy Markdown
Contributor

@rfecher rfecher May 24, 2016

Choose a reason for hiding this comment

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

perhaps a good idea - a req't for that compression is that for each periodicity/bin, the binary size must be fixed, but I don't think that should be a problem. preserving sort order actually shouldn't matter as we are just using the periodicity/bin as an enumeration...the more complex concern may be backwards compatibility?

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.

So what do we want to do here? I can come up with an approach to compress these a bit, which will require me to rework the parsing logic within getStartEpoch(), and and adjust the fixed binId sizes. Are we ok with this being a breaking change, or do we want to add complicated logic to handle both the new and old formats (a bit ugly)?

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.

I'd suggest you document an approach in an issue but don't worry about implementing it yet - tag the issue as "breaking changes"

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.

Opened #802

@dcy2003
Copy link
Copy Markdown
Contributor Author

dcy2003 commented May 24, 2016

@rwgdrummer rwgdrummer merged commit acb73a6 into locationtech:master May 27, 2016
@dcy2003 dcy2003 deleted the GEOWAVE-555 branch July 26, 2016 11:21
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