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

Featurization API tweaks #108

Merged
merged 3 commits into from
Nov 25, 2015
Merged

Featurization API tweaks #108

merged 3 commits into from
Nov 25, 2015

Conversation

bnaul
Copy link
Contributor

@bnaul bnaul commented Nov 19, 2015

  • Allow unequal-length multichannel data (passed in as lists of lists of 1d arrays), e.g. in the case of wavelet-transformed data
  • Add support for directly passing custom features as a dict {'some_feature': f} where f(t,m,e) is a featurization function, or as a dask graph {'some_feature': (f, 't', 'm', 'e')}.
  • Use default values for times/errors when omitted from featurize_time_series

Add `custom_functions` parameter to featurization functions which takes
a dict of functions to evaluate, or a dask graph of computations to
perform.
Tweak formatting standard for multichannel data: now multiple channels
w/ different lengths can be passed in as a list of lists of 1d arrays.
@acrellin
Copy link
Member

👍 from me

@profjsb
Copy link
Contributor

profjsb commented Nov 25, 2015

+1 from me

ERROR_VALUE = 1e-4
# Specify default values for missing times/errors
DEFAULT_MAX_TIME = 1.0
DEFAULT_ERROR_VALUE = 1e-4
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this default to NaN?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would like for it to, but currently a lot of the old TCP features will break if errors aren't passed in, or if they're taken to be zero. Even worse, we'd actually get different feature values if we changed this to 1e-1 or 1e-10; I'm just choosing 1e-4 to stay consistent with the old code.

@stefanv
Copy link
Contributor

stefanv commented Nov 25, 2015

Good job on the testing code, Brett. I'll add an issue on Travis coverage to ensure that we keep this up.

stefanv added a commit that referenced this pull request Nov 25, 2015
@stefanv stefanv merged commit ca75fe8 into cesium-ml:master Nov 25, 2015
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.

4 participants