scikit-learn-contrib / py-earth

A Python implementation of Jerome Friedman's Multivariate Adaptive Regression Splines
http://contrib.scikit-learn.org/py-earth/
BSD 3-Clause "New" or "Revised" License
457 stars 122 forks source link

Export functions #72

Closed ColCarroll closed 9 years ago

ColCarroll commented 9 years ago

Thanks for all the comments -- have been a little busy, so pushing changes I have so far. Point by point:

  1. Make func_factory private or change func_factory to be methods of BasisFunction subclasses (change _basis.pyx). _Added both func_factory and func_stringfactory as methods of subclasses
  2. Support SmoothedHingeBasisFunction (for smoothed models) - see section 3.7 (Degree of continuity) of Friedman, 1991(see README.md for reference) for details. This is done, and added to the tests
  3. Unit test for export_labelled_coefficients Removed this function for now. Do you have an idea for testing? Maybe just hardcoding in the correct coefficients.
  4. Match names for unit tests and the functions they test Done
  5. Update doc string for export_python_string to indicate it returns a string Done
  6. Maybe change name of export_python_model to export_python_function? Good idea. These functions are really in preparation to export a string in C.
  7. Does this support the constant term? It supports ConstantBasisFunction, if that's what you mean.
  8. Add function_name argument to export_python_model _I documented this a little better in the export_python_string method, but for export_python_model, I'm not sure what functionname would do.
  9. Move all this export related code into its own module, export.py. I think it will be easier to read and think about that way. _Good idea, also added test/test_export.py_
jcrudy commented 9 years ago

Thanks! Is this ready for merge as far as you're concerned? If so, I'll give it a more detailed look and try to merge it.

ColCarroll commented 9 years ago

Yes I think it is ready to merge.

jcrudy commented 9 years ago

Thanks, looks good. Just merged it.