Add support for measurement v4

JackAtOmenApps commented 4 years ago

Pretty excited for this update, and would like to include it in the latest version of my current project.

What are we still waiting for to merge this? Anything I can do to help? Thanks for the awesome work you've been doing, @codingjoe

gabn88 commented 4 years ago

Steps I had to take to make the new version work:

Change the MeasurementFields in the models. Add decimal_places and max_digits. replace measurement by measure, remove unit_choices (make a new Measure if you want to keep the unit_choices).
Change past migrations that added the MeasurementFields. import measurement.measures.mass becomes: from measurement import measures. And in the migration replace measurement=measurement.measures.mass.Mass with measure=measure.Mass.
In forms that used Weight, replace this by Mass. In forms that call weight.value, replace this by
Create new migration.
Migrate. Gives Out of range value for column 'weight' at row 8, this is because of the restriction of decimal places that is now added. Also now the default unit is gram instead of kg.

Since I need kilogram, or pounds, but it is only possible to store in the si_value (gram). max_digits and decimal_places won't work as expected. I thought about creating my own Measure (KgPoundMeasure) based on AbstractMeasure, but then the conversion will be harder (since no si_unit is included and also I can't get it to work, since I need to enter grams as well, to get to kilogram?). I'm a little bit lost now.

codingjoe commented 4 years ago

@gabn88 I believe it makes sense to specify in which unit we store the measures in a future version. Since decimals in Postgres and other database do not do decimal comma shifts to achieve precision on very high or low values. I will be working on it again, but right now I have to fight though my 50+ GH notifications.

gabn88 commented 4 years ago

@codingjoe Thanks for the quick response. No hurries, I was just investigating how the migration would look like.

I think it makes sense to implement something like: http://blog.elsdoerfer.name/2008/01/08/fuzzydates-or-one-django-model-field-multiple-database-columns/ When you store the value, as well as the chosen unit, it is also easier to show a default value on edit in the form, based on the chosen unit from last time.

According to: https://code.djangoproject.com/ticket/27 and https://code.djangoproject.com/ticket/5929 it should be easier since the _meta syntax on the django Model.

Just an idea, and since this update requires a big migration anyway it might be worth considering to also implement this.

codingjoe commented 4 years ago

@gabn88 hm… I agree that it makes sense to display whatever the user has selected in an interface. However on a database level, all columns in a row should be of the same unit. Otherwise indexing or quering will be a nightmare.

JackAtOmenApps commented 3 years ago

@codingjoe @coddingtonbear any progress toward merging this big improvement?

I have been working on a form field & widget more alike to the one in the current version of django-measurement, using MultiWidget & MultiValueField with the following features (among others):

Optionally specify the primary/default unit to be displayed from among unit_choices. If not provided, the base unit is used. This should resolve at least part of the issue @gabn88 brought up. Nothing new is stored in the table, but on each form field, you can specify the 'default' unit (and by association, the appropriate converted value) to be shown to the user.
Optionally display a table which shows conversions to each of the units in the field's unit_choices. This makes it so much easier to make changes in apps that regularly make use of a few different units.

Also, since PostgreSQL can store Decimals of arbitrary length, I have another PR nearly ready to go which adds that option for the model field, based on the database engine selected in the project settings. With this PR, if we use Postgres, then we can optionally just not worry about max_digits and decimal_places and let the db worry about that.

But I don't think it makes sense to send these PRs based on another PR which hasn't yet been merged. (Is that accurate?)

JackAtOmenApps commented 3 years ago

Just verified that my modified form widget (based on this PR) will also resolve #101

codingjoe commented 3 years ago

Hi @OmenApps,

Thanks for our all hard work the last couple of days. Sadly, I currently don't use this package in a commercial context, with very much how much time I can sink into this. That being said, you seem to have the motivation and skill to push this over the finish line. I'd more than welcome a working patch from you. You can use my work as a start if you want, or don't 🤷

That aside, I would like to endorse you as a new maintainer, if that is something you are interested in? Just let me know, and I will write an email to Adam. I am sure he has no objections.

Best, Joe

JackAtOmenApps commented 3 years ago

@codingjoe

I'll get working on a patch, based on the work you've done.

It would be an honor to help out as a maintainer. I am working to implement this package in one of my commercial products, so I should be able to dedicate some time consistently to help keep it up-to-date and bug-free. I just want to make sure I keep to the high standards you and Adam expect.

codingjoe commented 3 years ago

@codingjoe

I'll get working on a patch, based on the work you've done.

It would be an honor to help out as a maintainer. I am working to implement this package in one of my commercial products, so I should be able to dedicate some time consistently to help keep it up-to-date and bug-free. I just want to make sure I keep to the high standards you and Adam expect.

Hi @OmenApps, welcome on board! I let me know if you can help you with a review. You may close this pull-request at any time, should you no longer have any use for it. Best, Joe

JackAtOmenApps commented 3 years ago

Been a bit busy the past couple weeks, but I intend to have the new PR for using measurement v4 by this weekend (at which point I'll close your PR).

bondarev commented 3 years ago

Been a bit busy the past couple weeks, but I intend to have the new PR for using measurement v4 by this weekend (at which point I'll close your PR).

@OmenApps Hello!

Any news?

JackAtOmenApps commented 3 years ago

@bondarev it's slow going. I have a (not polished, properly tested, or fully documented) fork I'm using successfully in a project, but it needs significant work before it'll be ready to push.

I hope to have time soon. It's important to me, but I have been on-boarding new clients in my business, which has really taken more time than expected recently. No promises on time, but I have not forgotten this project.

mikeford3 commented 2 years ago

Hi @OmenApps

Any chance of an update on this? Thanks, Mike

omdathetkan commented 2 years ago

@OmenApps this would also be super useful for me! Is this still alive?

gabn88 commented 1 year ago

Since now is the time of the year to look forward for next year I would like to help get the release out that supports v4. What work needs to be done?

Especially since python-measurement v3 does not install on my new laptop, but v4 does ;)

coddingtonbear / django-measurement

Add support for measurement v4 #99