Skip to content

Add 'SlugSquareFoot' and 'SlugSquareInch' as units of MassMomentOfIntertia, and 'slug' as a unit of Mass #482

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

Merged
merged 2 commits into from
Oct 12, 2018

Conversation

McNeight
Copy link
Contributor

No description provided.

Copy link
Owner

@angularsen angularsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good, but I found some minor differences in the conversion factors you should look over.

"SingularName": "Slug",
"PluralName": "Slugs",
"FromUnitToBaseFunc": "x/6.85217659e-2",
"FromBaseToUnitFunc": "x*6.85217659e-2",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tiny rounding error, I believe it is 6.852176556196105e-2 according to
https://www.convertunits.com/from/slug/to/kg
https://en.wikipedia.org/wiki/Slug_(unit).

{
"SingularName": "SlugSquareFoot",
"PluralName": "SlugSquareFeet",
"FromUnitToBaseFunc": "x*1.35583",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

{
"SingularName": "SlugSquareInch",
"PluralName": "SlugSquareInches",
"FromUnitToBaseFunc": "x*9.41548e-3",
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@angularsen
Copy link
Owner

@McNeight Just a follow up, I'm waiting for your feedback on how to proceed here.

@angularsen angularsen self-assigned this Oct 1, 2018
@angularsen
Copy link
Owner

@McNeight I don't mean to nag, but still waiting for you on this one. Only minor changes required to merge this.

@McNeight
Copy link
Contributor Author

@angularsen I apologize for the delay, but I've made the changes you've requested.

For future reference, what are good sources for conversion constants, and how many significant digits do you prefer?

@angularsen
Copy link
Owner

Awesome!
https://github.com/angularsen/UnitsNet/wiki/Adding-a-New-Unit#quick-summary-of-steps

I addet a new bullet point to the list:

For irrational (infinite) conversion factors, use 7 significant figures

Good sources are google and various conversion websites. I usually double check with two sources.

In the wiki there is an example on how to look up these factors.
https://github.com/angularsen/UnitsNet/wiki/Adding-a-New-Unit#4-fix-generated-test-stubs-to-resolve-compile-errors-tests

@angularsen
Copy link
Owner

angularsen commented Oct 12, 2018

I changed the wording to

For irrational (infinite) conversion factors, use as many as you can up to 12 significant figures

Copy link
Owner

@angularsen angularsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks

@angularsen angularsen merged commit ebf78f2 into angularsen:master Oct 12, 2018
McNeight added a commit to McNeight/UnitsNet that referenced this pull request Oct 12, 2018
@angularsen
Copy link
Owner

@McNeight
Copy link
Contributor Author

Awesome! I just realized that I forgot to modify the test cases. It looks like the tolerance was loose enough that they passed. If it's easier, I can open a second issue to merge that in.

@angularsen
Copy link
Owner

Yes please, good to have it in I think

angularsen added a commit that referenced this pull request Oct 12, 2018
Updated conversion constants in test cases from #482
@angularsen
Copy link
Owner

Released in 3.107.1.

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

Successfully merging this pull request may close these issues.

2 participants