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

Unit for "Amount of times" #849

Closed
selmaohneh opened this issue Oct 14, 2020 · 16 comments · Fixed by #889
Closed

Unit for "Amount of times" #849

selmaohneh opened this issue Oct 14, 2020 · 16 comments · Fixed by #889

Comments

@selmaohneh
Copy link

Is your feature request related to a problem? Please describe.
I have a property of type IQuantity that users of my framework can fill as they desire. Often the value represents a technical number with a corresponding unit.
But sometimes the value should represents a simple count of how often something was done. This quantity would be unit-less although being numerical.
Therefore the users have no way to generate the quantity, since the unit is a required argument of i.e. Quantity.From(...).

Describe the solution you'd like
A QuantityType called "None" would fit my needs. It should contain a single unit called "Times", e.g. [1].

@angularsen
Copy link
Owner

angularsen commented Oct 14, 2020 via email

@selmaohneh
Copy link
Author

Frequency does not work, since it has a time component in it, i.e. times per second.
Ratio would work, since it is unit-less. Still it is not that intuitive, since a ratio implies a relationship between two numbers.

Temperature.From..., Pressure.From... and similar methods force the user to think about what type of quantity they want to create - that's good API design. It's easy, concise and leads the user to use UnitsNet correctly (pit of success). Telling the user to use Ratio for a "count"/"number of times" is misleading and not self-explaining (e.g. I had to open this issue first :D).

In my opinion the QunatityType "None" with unit "Times" would therefore be a good enhancement, even if just for API purposes.

What do you think?

@angularsen
Copy link
Owner

angularsen commented Oct 17, 2020

You are not the first to ask, and the answer has typically been to use double, Ratio or Frequency.
Is there a particular usecase where having this quantity would be more useful to you than double or Ratio?

Since people keep asking about it, I suppose we could add it.
How about something like this:

  • Quantity Amount with units:
    • Count (ideally long, but probably decimal like Information or double like the majority of the quantities use)
    • Thousand
    • Million
    • Billion
    • etc..
Amount.FromMillons(1.5).Count; // 1.5e6
Amount.FromCount(1500).Thousands; // 1.5

Not to be confused with https://github.com/angularsen/UnitsNet/blob/master/Common/UnitDefinitions/AmountOfSubstance.json

Since we don't know what we're counting, I feel decimal may be a safer choice. Although, we already have some issues with supporting both decimal and double for things like nanoFramework (#836) and WRC (#815).

@selmaohneh
Copy link
Author

I am taking the self-explanatory API position again:

var numberOfSheep = Amount.FromCount(42);

reads very good. This would not be the case when the word Ratio is somewhere in that line... So I'd say you should go for the new type and unit.

I guess .FromCount is enough. The other creation methods might lead to missusages. C# already has enough syntax sugar for creating big numbers, i.e. 42e6.

Not sure about the type, tho. Count always sounds like an int inside the .NET world. Counts like 2.3 and 4.23 do sound more like they could fit inside Ratio since they actually are one. 🤔

@angularsen
Copy link
Owner

I agree 1.5 "count" does not make much sense, integers would be more natural there - otherwise use Ratio as you say.

Alternative names:

var amount = Amount.FromUnits(42);
amount.Units; // 42

var amount = Amount.FromSingles(42);
amount.Singles; // 42

var amount = Amount.FromOnes(42);
amount.Ones; // 42

var amount = Amount.FromIndividuals(42);
amount.Individuals; // 42

I would argue to keep units of thousands, millions etc. to benefit from these:

var amount = Amount.FromOnes(4200000);
var amountInMilions = amount.Millions; // 4.2
var millionString = amount.ToUnit(AmountUnit.Million).ToString(); // 4.2 M

// as opposed to
var amountInMilions = amount.Ones / 1e6; // 4.2
var millionString = $"{amount.Ones / 1e6} M"; // 4.2 M

@selmaohneh
Copy link
Author

Okay, I see the benifit of the helper methods. 👌

Don't like the alternative names (for the default creator method) tho.

My favorites would be FromValue, FromCount or FromAmount.

@angularsen
Copy link
Owner

angularsen commented Oct 18, 2020

I think I like FromValue and .Value the best. @tmilnthorp @lipchev any thoughts on this suggestion overall?

@selmaohneh If we agree on this, would you be interested in attempting a pull request on this? I'm happy to assist.

@selmaohneh
Copy link
Author

selmaohneh commented Oct 19, 2020

I'll come back to you later today. I'll have to talk to my colleague about this. If we decide to go the UnitsNet way, I can work on the PR. We would need that functionality in around 3 weeks then. 🙄 Is that goal realistic?

@angularsen
Copy link
Owner

Great, yes 6 weeks is plenty.

Detailed steps here:
https://github.com/angularsen/UnitsNet/wiki/Adding-a-New-Unit

It is pretty easy once you understand how the code generation based on the JSON files works, and there are a lot of existing quantities to compare to.

@selmaohneh
Copy link
Author

Yep, already read that doc. Just wanted to make sure we won't have huge loose ends shortly before the deadline because we aren't able to get a UnitsNet-PR merged. :D

@selmaohneh
Copy link
Author

We decided that switching to UnitsNet is out of scope for our sprint. So right now I won't find the time to start implementing it. Freetime is daughter time currently. 😐😊

@tmilnthorp
Copy link
Collaborator

This is really just the need to have double/float/etc in an IQuantity format.

I would call it Scalar or Dimensionless and FromValue/Value does seem to be the best fit.

@angularsen
Copy link
Owner

No problem @selmaohneh , let us know if you want to continue this.
I like Scalar @tmilnthorp 👍

@stale
Copy link

stale bot commented Dec 19, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Dec 19, 2020
@stale stale bot closed this as completed Dec 26, 2020
@Joe-houghton
Copy link
Contributor

I know this has been closed as stale, but I wanted to add support for this feature.
Our application allows users to state how many plants to put in an area / along a fence. Users can for example specify 12/m², 1600/Hectare and various other density options, but they also have the ability to say 15 plants go in the area.
It would make our codebase much messier to have multiple functions + if statements, or a wrapper around UnitsNet. As tmilnthorp suggested we really just need "to have double/float/etc in an IQuantity format.".

I did take a look to see if I could do the work, but got stuck at the first hurdle, where BaseUnits are in 7 categories and this one doesn't really fit in any (could possibly squeeze it into a length, but felt hacky).

@angularsen
Copy link
Owner

angularsen commented Jan 4, 2021

No I agree, if you are interested in doing a pull request then I'm happy to help.

The way I see it, we have two options:

  1. Add Scalar.json, similar to how it's done for Ratio.json. See Wiki for instructions.
  2. Implement Scalar as custom code without code generation, similar to how the example quantity HowMuch was done.

I sort of think it could be slightly useful to add units "Thousands", "Millions" etc.
It would make this possible out of the box: `Scalar.FromMillions(1.5).ToString() // "1.5 M"

However, I'm also fine with letting this kind of numeric text representation be a concern for Humanizer.

As for BaseUnits, see how Ratio did it. Many quantities don't map to SI base units.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants