Add Reactive and Apparent power quantities

[ela-kotulska-frequenz]

No description provided.

[ela-kotulska-frequenz]

@llucax
Pylint is falling with error:
tests/test_quantities.py:1:0: C0302: Too many lines in module (1092/1000) (too-many-lines)
I can either:

1. ignore this error : pylint disable too-many-lines
2. separate tests into: test_quantities_base, test_power, test_frequency itd... This also means separating methods like test_zero - which checks if all quantities zero() method.

I think 1) is ok, because the tests seems descriptive and readable now, and I don't see reason to repeat the same function definitions in every file.

What do you prefer?

[llucax]

Ideally I would prefer, now what each quantity lives in its own file, to have one test file per quantity, but I think it was very difficult to split the test as it is structured now. I don't think it is worth investing a lot of time on this, at least for now, so based on how complicated you see the split (maybe you could try to ask ChatGPT to do the split, for these types of things I think it can be pretty effective) or just add an ignore.

[llucax]

I'm assuming the code is mostly a copy&paste from Power, so not reviewing much in details. I think things get complicated with multiplication and division. From a quick search I find mentions of VAh and VARh as the units for energy coming from apparent and reactive power respectively. I'm not sure if we should introduce also ApparentEnergy and ReactiveEnergy 😬

Maybe @tiyash-basu-frequenz @matthias-wende-frequenz or @cwasicki know better?

[llucax]

Same here, I wouldn't remove this until 1.0 final.

[ela-kotulska-frequenz]

Done

[cwasicki]

I'm not sure if we should introduce also ApparentEnergy and ReactiveEnergy

At least atm I am not aware of a use-case for this.

[llucax]

At least atm I am not aware of a use-case for this.

You mean you won't need to get energy from reactive or apparent power? If we don't have uses cases for this and we are not sure what to return when multiplying by time, I would not implement the multiplication by time for these 2 new units for now, we can add them later when we have more clarity, as adding will be a non-breaking change but removing or changing will.

[cwasicki]

Just saying that I don't know of an app which is using this currently, so adding it later sounds reasonable to me.

[ela-kotulska-frequenz]

• We need reactive power for the new formula GridReactivePowerFormula
• The problem with current quantity Power is that it will print 100W instead of 100VAR. The same for ApparentPower : it will print 100W instead 100VA. It is misleading.

[llucax]

@shsms @ela-kotulska-frequenz do we really need to use __mul__ with timedelta now for these new quantities? Otherwise I strongly suggest removing those. We introduced an asymmetry with this: REACTIVE POWER * TIME / TIME = POWER (not reactive).

I think this is not good, and if we really need this we should think about it more. Let's discuss before a release with these new quantities.

[ela-kotulska-frequenz]

@llucax __mul__ with timedelta has been removed. We don't use it in new quantities.

Please look at last commit in this PR, "Remove the Energy interactions for ReactivePower and ApparentPower"

[Marenz]

I am not sure if it s out of scope for this PR/repo, but an explanation or at least a link to what Reactive/Apparent power is in the first place would be nice, no?

[ela-kotulska-frequenz]

@Marenz
The definitions of Reactive and Apparent power are well defined physical quantities.
I don't think we need definitions in code, as we also don't need them for other quantities here like: Frequency or Current or Power.

But if I am wrong, I can write something in follow up PR

[Marenz]

Maybe they are well defined, but isn't our target audience not a non-hard-core programmer than can do python stuff and supposedly doesn't have and education in physics? (I never heard of those quantities before I started here for example and actually still don't know what they are).
Or why they justify a whole own unit, each..

If we expect our target audience to know this, then of course I withdraw my point :)

[tiyash-basu-frequenz]

Documenting active/reactive power is a good thought, but it open up further issues, I feed. Then we need to document every other electrical metric, and we need to document them every time they are used or mentioned.

Maybe it is better to look into adding the definition of these terms to our glossary (in a separate issue).

but isn't our target audience not a non-hard-core programmer than can do python stuff and supposedly doesn't have and education in physics?

I thought the audience was just novice-programmer. Non-physics-educated sounds new to me. I would have thought that the audience needs to have some knowledge of electricity-related physics, otherwise they would anyway have zero idea of what to do here.

[Marenz]

I mean, I was able to string a long pretty far without knowing those two things :P
The other stuff (Currency, Power, Frequency) is a lot more common imo, not sure how many people even heard of apparent/reactive power

[llucax]

@llucax __mul__ with timedelta has been removed. We don't use it in new quantities.

Please look at last commit in this PR, "Remove the Energy interactions for ReactivePower and ApparentPower"

Ah, great, sorry for the noise, I must have been looking at an old state of the PR.

[llucax]

About documenting what each quantity is, I think a link to Wikipedia should be enough, no? Very low effort and useful enough for the people that don't know and want to learn.