Skip to content

Radices#475

Merged
swharden merged 10 commits intoScottPlot:masterfrom
bclehmann:radices
Jul 5, 2020
Merged

Radices#475
swharden merged 10 commits intoScottPlot:masterfrom
bclehmann:radices

Conversation

@bclehmann
Copy link
Member

@bclehmann bclehmann commented Jun 23, 2020

New contributors should review CONTRIBUTING.md

Purpose:
#469

New functionality (code):
Provide a code example demonstrating new functionality achieved with this pull request (if applicable):

//Base 10 on x axis, base 16 on y axis
int[] radices = { 10, 16 };
string[] prefices = { "", "0x" };

plt.Ticks(radices: radices, prefices: prefices);

It also adds the ScottPlot.Tools.ToDifferentBase method:

 public static string ToDifferentBase(double number, int radix = 16, int decimalPlaces = 3, int padInteger = 0, bool dropTrailingZeroes = true)

There are some tests for this method in ScottPlotTests.Tools.ChangingBase

New functionality (image):
image

Autoformat your code:
The build will fail if your code is not auto-formatted. Auto-format your code in Visual Studio, or from the console using these commands:

cd ScottPlot/src/
dotnet tool install --global dotnet-format
dotnet format

@swharden
Copy link
Member

Wow @Benny121221, what an interesting idea! And clean implementation (considering the tick system is a little clunky). Thanks for this!

I'd like to add a title, vertical axis label, and horizontal axis label to the demo to make the use case more obvious. What do you think a good labels would be to communicate when this would be used in practice? Memory addresses come to mind, but perhaps those would go on the horizontal axis, so I'm interested in your thoughts about what would make a good demo use case.

@bclehmann
Copy link
Member Author

I think a good demo would be stacked bar-plots to show memory usage (e.g. 0x40000000 to 0x400F0000 are allocated to process A, 0x40100000 to 0x50000000 are for process B, etc.

I admit my demo was a little lazy and unimaginative, seeing as I just drew a straight line.

@swharden
Copy link
Member

I think a good demo would be stacked bar-plots to show memory usage (e.g. 0x40000000 to 0x400F0000 are allocated to process A, 0x40100000 to 0x50000000 are for process B, etc.

I admit my demo was a little lazy and unimaginative, seeing as I just drew a straight line.

Sounds good to me! I'll let you update the demo to reflect the multi-process memory idea, then let me know when you think this PR is done and ready to merge

@bclehmann
Copy link
Member Author

image

The demo isn't quite as neat as I thought it would be, I think partly because the tick system favours nice numbers in base 10, not in base 16. I don't think this is really worth fixing, especially as there aren't really any nice numbers in base 16 other than full bytes. I expect anyone who was so nitpicky would label the axis themselves anyways.

I think this is all that's needed for this PR, so feel free to merge

swharden added 3 commits July 5, 2020 12:04
previously arrays were passed in, now X and Y arguments can be specifically and independently defined
@swharden
Copy link
Member

swharden commented Jul 5, 2020

I'm about to merge this in, 2 thoughts though.

  • Should we throw if the base is >16? char[] symbols = "0123456789ABCDEF".ToCharArray(); makes me think it would crash anyway, so maybe we should make it crash up-front with a descriptive exception message
  • Negative values always show as 0. Is this intentional?

@bclehmann
Copy link
Member Author

  • Should we throw if the base is >16?

Yeah we probably should.

  • Negative values always show as 0. Is this intentional?

No that is not intentional, good catch. I should have included tests with negative values. This does raise a problem as some bases (binary) have multiple ways of writing negatives (one's complement, two's complement, sign and magnitude, etc). I think showing that would be confusing and harmful, so I think it makes the most sense to prepend a negative sign.

@swharden swharden merged commit ef3017c into ScottPlot:master Jul 5, 2020
@swharden swharden mentioned this pull request Apr 10, 2021
48 tasks
@swharden swharden mentioned this pull request May 17, 2021
82 tasks
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