Closed f0ma closed 3 years ago
Hi, I think it needs sufficient refactoring to be published in PIP. In it's current form the repository is more of a example/experiment than a library with a well-designed API. Should plots be included in the library? Should (rather unconventional) data regularization procedures remain? I did not find a suitable answer, so decided to leave it as it is. If you have some thoughts, I am open to discussion.
Following text is just my habble opinion: Today as I know is no packages in PIP for:
All thouse useful methods available in ellipsoid_fit.py
File have no dependencies except numpy and optional matplotlib.
About API I think ellipsoid_fit
and ellipsoid_plot
already have a stable API, but data_regularize
require optional aggregation method as argument (default np.mean(np.array(points_in_sector), axis=0)
, also "Other strategy ..." code, may be some problem specific method by user chouse).
And tests...I think good idea to create some synthetic dataset to make smoke test. I am ready to prepare test data and unittests and share some real data for making a tests.
About tests - I actually did create synthetic datasets while fixing issues a couple of years ago, just did not commit it. The biggest problem with this repo is - I don't actually use it and created it in a day or so to calibrate an mpu9150 I had in 2015.
Some doubts I have are
Overlall I agree, that there are some useful functions, and I am happy that someone uses the code. Even disregarding all the concerns above, those useful functions you mentioned are mostly related to each other by their connection to magnetometer calibration, so should the package be also called something like 3d_ellipsoid_calibration?
And another problem with "spherical regularization" is that a center for sphere is a wild guess int the style (min + max) / 2.
Maybe the perfect solution would be join with mentioned minililnim's repository into comprehensive "3d_ellipsoids" package. Or leave everythin as is - it's still googlable.
Do you have a plan to publish ellipsoid_fit_python in PIP package repository? For example in PIP already available package for ellipse fitting (https://github.com/bdhammel/least-squares-ellipse-fitting).