Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Discussion options

Would it be possible to have Mach added to the SpeedUnit conversions. I realize it is an odd unit for conversion since Mach depends on temperature and pressure and possibly other factors. I think there is a standard for conversion to use 1 atm at 20C. What do you think?

You must be logged in to vote

Replies: 2 comments · 5 replies

Comment options

Hi, sure I think this sounds reasonable as long as people in this domain generally agree that these standard conditions are the most commonly used to represent Mach.

Checking with some other online converters, they seem to agree on the standard conditions.
https://www.metric-conversions.org/speed/mach-to-kilometers-per-hour.htm
https://www.convertunits.com/from/mach/to/km/h

Would you be interested in attempting a pull request?

Steps are outlined here, and I'm happy to assist.
https://github.com/angularsen/UnitsNet/wiki/Adding-a-New-Unit

You must be logged in to vote
3 replies
@tjrtfarnham
Comment options

Hello Andreas,

I am getting a testing error in Speed_QuantityInfo_ReturnsQuantityInfoDescribingQuantity with Assert.Equal(units, quantityInfo.Units);

units:
image

quantityInfo.Units:
image

  1. It seems strange to me that the generated SpeedUnit value is "34" and not 33.
  2. Within one list the Mach value is sorted among the other units, and the other list it is not.

Hoping you might have some help on how I can fix this issue, or where I might be going wrong.

Also, do I generate a pull request including the generated files, or only those I have directly modified?

@tjrtfarnham
Comment options

I tried naming the unit to "ZMach" instead of "Mach" in the json file. This cleared up the testing error and gave the ZMach speed unit enum a value of 40 instead of 34. It obviously needs to be labeled "Mach", so hopefully I am doing something stupid.

@angularsen
Comment options

@tjrtfarnham Sorry for the delay.

  1. The enum values are intentionally random, to try to avoid merge conflicts.
  2. The ordering is correct, but should also not matter much, unless this is breaking your test?
  3. Yes, please create a pull request with all the generated files. It will be a lot, but we have configured github to collapse most of the generated stuff so we can more easily focus on the actual changes.

Let's continue the conversation in the PR, so I can better see what the test error is about. We recently changed how enum values are "allocated" so they remain consistent when adding new units later, it might be relevant.

Comment options

I will give this a shot. Hopefully will have a pull request within a couple of days. Thanks.

You must be logged in to vote
2 replies
@tjrtfarnham
Comment options

@angularsen When I try to push my branch, I get a permission to angularsen/UnitsNet.git denied to tjrtfarnham. Maybe I need permissions added to push the changes for my branch? Or maybe I'm not following some rules that I should be?

@angularsen
Comment options

Sorry, yes there is a rather steep learning curve to GitHub, forks and pull requests.

I just jotted down some bullet points in the wiki on how to create forks and pull requests, if you are not familiar with this.
https://github.com/angularsen/UnitsNet/wiki/Adding-a-New-Unit#6-create-pull-request

Also, here are some more official docs:
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-a-pull-request

Please let me know if you still run into problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Category
💡
Ideas
Labels
None yet
2 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.