search

nholthaus / units

a compile-time, header-only, dimensional analysis and unit conversion library built on c++14 with no dependencies.
http://nholthaus.github.io/units/
MIT License
955 stars 135 forks source link

V3.x simplify conversion factors #256

Closed nholthaus closed 3 years ago

nholthaus commented 4 years ago

This PR removes the symbols naming conversion factors for each unit container type.

JohelEGP commented 4 years ago

The change is really hard to see. Can you rebase it on v3.x?

nholthaus commented 3 years ago

rebased to v3.x. coveralls isn't working (maybe hasn't been for a while?) but I don't want that to hold up the rename, and I think it would be better to solve it in another PR.

JohelEGP commented 3 years ago

What rename? The units were renamed and this removes the conversion factors. I can tell you did that from the tests, but I can't see how you did it because the diff for core.h is messed up.

nholthaus commented 3 years ago

Whitespace issue, I was hoping the rebase would help. I'll take care of that.

You can view the diff without the whitespace change

You're right, not a rename, it's a removal.

JohelEGP commented 3 years ago

How are the error messages after this? meters is actually unit<...>, so I suspect they'll revert the compiler to being a novelist.

nholthaus commented 3 years ago

I need to check, when I started we still had strong units.

That said, conversion names didn't appear in the error messages as often a you might think, hence why we used to see all the old SI dimension ratios.

On Wed, Oct 7, 2020, 8:57 PM Johel Ernesto Guerrero Peña < notifications@github.com> wrote:

How are the error messages after this? meters is actually unit<...>, so I suspect they'll revert the compiler to being a novelist.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/nholthaus/units/pull/256#issuecomment-705269732, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACOYOH4P2O4US3VV2BSRDALSJUE7VANCNFSM4SGTZWGQ .

JohelEGP commented 3 years ago

That said, conversion names didn't appear in the error messages as often a you might think, hence why we used to see all the old SI dimension ratios.

Seems like I didn't implement the follow up to identify strong units within ephemeral units. I suspect it's not easy to do, anyways.

nholthaus commented 3 years ago

The latest push keeps the readability improvements in the definition, while retaining the improved error messages. Clang is a bit inconsistent about when it uses named strong types, but overall still improved.

We weren't able to get rid of any symbols, but at least they've become a pure implementation detail.

I think this is ready to go.

nholthaus commented 3 years ago

resolves #209