-
Notifications
You must be signed in to change notification settings - Fork 393
Suggestion: Angle (Degrees) to N-S-E-W, #585
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
Comments
Hmm. I don't know enough about this domain. Is North 0 degrees and West 90 degrees, or something like that? Is that definition standardized worldwide, or does it vary for different countries/usage domains? At first I thought, sure, let's just add them as angle units. But then I saw some issues:
It took me a moment, but I'm now realizing that NNE is not a unit. It's a quantity of angle. We can, however, add something like this: Angle nnw = Angle.NorthNorthWest; // Static readonly getter property for new Angle(22.5, AngleUnit.Degree) or whatever the value is
double nnwDegrees = nnw.As(AngleUnit.Degree); // 22.5
string nnwText = nnw.ToString("Wind is blowing {0} {1}."); // "Wind is blowing 22.5 º." I don't think we should add logic to convert say Am I understanding this right? |
We might also be be able to add something like this, which is probably more what you asked: WindDirection wd = Angle.GetWindDirection(90); // WindDirection.West enum value It does, however, smell like app-specific code to me. Some only care about N,W,E,S, others want N,NW,W,SW,S etc.. I can imagine others want three-letter directions. Let me hear what you think first and maybe I misunderstood some things :-) |
That would be great. Perhaps like FeetInches it could contain subvalues, like compas.major = North, compas.minor = NorthWest, compas.full = NorthNorthWest Just a thought, I have only used your library for 10 minutes, so I think you're better qualified to implement it the best way, if you agree it should be include. Btw, Great library !! |
Btw, nice overview of how it all works here: |
One more thing, IF you decide to do it... Perhaps consider adding a 'reverse' direction, so if I have for example SSE, the reverse would be NNW. Useful if you are figuring out for example a wind blowing in a SSE direction, that means it's coming from a NNW direction. Yeah, I'm into wind these days, what can I say :) |
Ok, I'm sold. Let's add this thing and I want you to add it :-) We need to come up with a design proposal first. I'll toss in what I can think of from the top of my head, but please revise this and change as you see fit. Nothing long or fancy, just paint a picture for how it will be used and what new methods, properties or types we need. Angle.ToString(CompassType)Added via Angle a = Angle.FromDegrees(33.8);
a.ToString(CompassType.Points32)` // NEbN
a.ToString(CompassType.Points16)` // NNE
a.ToString(CompassType.Points8)` // NE
a.ToString(CompassType.Points4)` // N Angle factory methodsAdded via Angle a = Angle.FromCompassPoint(CompassPoint.NNE); // 22.5 degrees Reverse compass angleNote: All compass angles should be 0-360 degrees, not negative nor larger than 360. Angle nne = Angle.FromCompassPoint(CompassPoint.NNE); // 22.5 degrees
Angle ssw = nne.ReverseCompassAngle(); // 202.5 degrees - is there more generic term/name we can use here that is not restricted to "compass" but circles in general? New types
|
How about this? Angle.ToString(CompassType)What you suggested is great, but any reason we can't also add this? -- These may be too domain specific ? : So, with a ToString(string) function:
Otherwise your suggestion is fine too, I guess I like the option so people can format it the way they want it. Opposite compass angleThe Opposite side of a circle I believe the term is after some googling.. Angle nne = Angle.FromCompassPoint(CompassPoint.NNE); // 22.5 degrees New typesenum CompassPoint { NNE, NbE /* etc... */ } See also: https://en.wikipedia.org/wiki/Compass_rose I think that's pretty much my suggestions... Feel free to kill the ones that may not fit the package. |
Sure, I guess we can add format pattern support. We haven't done that before, so you might have to pave some new road here. Basically, all ToString() methods are generated by PowerShell scripts, so I think we'll have to modify the code generator to insert something like this: public string ToString([CanBeNull] IFormatProvider provider, [NotNull] string format, [NotNull] params object[] args)
{
if (format == null) throw new ArgumentNullException(nameof(format));
if (args == null) throw new ArgumentNullException(nameof(args));
// BEGIN NEW CODE
foreach (var formatter in _toStringFormatters)
{
format = formatter.Format(format, args);
}
// END NEW CODE
provider = provider ?? GlobalConfiguration.DefaultCulture;
var value = Convert.ToDouble(Value);
var formatArgs = UnitFormatter.GetFormatArgs(Unit, value, provider, args);
return string.Format(provider, format, formatArgs);
} Then we can in We also need to document these format patterns, at minimum in the xmldoc so you can see all options with intellisense when coding. It's also possible there are some hints we can give to ReSharper and other tooling to understand that some characters are format pattern characters, not sure, maybe they just hard coded support for dates and time patterns. Just look into it and see what the best practices are. Snipped from: UnitsNet/UnitsNet/GeneratedCode/Quantities/Mass.NetFramework.g.cs Lines 960 to 970 in 5542558
|
Cross-reference #579 |
@TheMightyPope Please take a look at #579 as that PR is nearing completion now, for a reference on how to do format strings. Do you still intend to follow up with a PR on this issue? |
Trying to find time, I've never actually even used github before, and my
startup is taking all my time. Might not make it for PR.
…On Mon, Feb 25, 2019 at 4:58 PM Andreas Gullberg Larsen < ***@***.***> wrote:
@TheMightyPope <https://github.com/TheMightyPope> Please take a look at
#579 <#579> as that PR is
nearing completion now, for a reference on how to do format strings. Do you
still intend to follow up with a PR on this issue?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#585 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAgHm-hKIgaN8YY_GaEsdiP7A1g34GSOks5vRIa5gaJpZM4aPrsW>
.
|
I understand, I'll leave it open for a few weeks and check back if I don't hear anything. |
It's been a few weeks now, do you still intend to follow up on this or is it better to close it for now? |
Closing due to inactivity. |
When using for example the National Weather Data API you will see data for Wind Direction expressed in Degrees, like this:
[https://api.weather.gov/gridpoints/LWX/100,60
...
It would be useful to convert this to a wind direction expressed as North or N, North North-East (NNE), etc, and of course the other way too.
The text was updated successfully, but these errors were encountered: