Specs needed before any further work is done

[translunar]

I've spent some time looking at your code, and I think that fundamentally you haven't done enough design.

I want you to pause your regular coding right now and switch over to writing specs.

Your first order of business is writing a spec helper that does image differencing in order to tell you if you've broken some feature in plotting. I want you to show me what you come up with before you continue coding.

CC @v0dro

[v0dro]

Works. I think the image differentiation spec will be very coupled to image magick so let it be in this repo.

However, I am of the opinion that common specs should be written in the rubyplot repo for better co-ordination: https://github.com/SciRuby/rubyplot/tree/master/spec

[translunar]

I think that’s fine, but I want him to consider all the possible plots he needs to make as he continues with his coding. So if the specs need to go in a different place, that’s fine, so long as he does them and starts applying them.

…

On Tue, Jun 19, 2018 at 9:30 PM Sameer Deshmukh ***@***.***> wrote: Works. I think the image differentiation spec will be very coupled to image magick so let it be in this repo. However, I am of the opinion that common specs should be written in the rubyplot repo for better co-ordination: https://github.com/SciRuby/rubyplot/tree/master/spec — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#4 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAKFpLdwj9ggu_cFDNEPlFgCXpRk7K_Cks5t-c_6gaJpZM4Uulx4> .

[pgtgrly]

Greetings @MohawkJohn @Arafatk
@v0dro and I just had a coding session where we discussed the Scripting layer based on the discussionhere

We came up with certain specs that based on the discussion to model the scripting layer.This might help you design your specs.

require 'spec_helper'

describe Rubyplot do
  before do
    @x1 = [1, 2, 3, 4, 5]
    @y1 = [10, 20, 30, 40, 50]
  end
  
  context "#line" do  
    it "creates a simple line graph" do
      a = Rubyplot.new
      a.line @x1, @y1 
      a.save "file_name.bmp"

      expect(equal_files("file_name.bmp", "line_graph.bmp")).to eq(true)
    end

    it "creates a line graph with points marked" do
      a = Rubyplot.new
      a.line @x1, @y1, points: true
      a.save "file_name.bmp"

      expect(equal_files("file_name.bmp", "line_points_graph.bmp")).to eq(true)
    end
  end

  context "#scatter" do
    it "creates a simple scatter graph" do
      a = Rubyplot.new
      a.scatter @x1, @y1
      a.save "file_name.bmp"

      expect(equal_files("file_name.bmp", "scatter_graph.bmp")).to eq(true)
    end
  end
end

require 'spec_helper'

describe Rubyplot do
  before do
    @x1 = [-10, 0 , 5, 28] 
    @y1 = [1, 2, 3, 4]
    @x2 = [2, 4, 16]
    @y2 = [10, 20, -40]
  end

  context "#line #scatter" do
    it "creates a line and scatter graph" do
      a = Rubyplot.new
      a.line @x1, @y1
      a.scatter @x2, @y2
      a.save "file_name.bmp"

      expect(equal_files("file_name.bmp","line_scatter_graph.bmp")).to eq(true)
    end
  end
end

I have put them here so that they can be viewed even if the files are removed. Currently, They can be found here and here.

[translunar]

These are beautiful; excellent work.

I only have a few comments.

I like the name Rubyplot and think we should use this for the library. However, I don't think we should use this for the plots themselves. It would probably be better to stick to Figure or even Axes like in Python.

By the way, one of my most frequently used features in matplotlib is fig.add_subplot(...), and I'd like you to think a bit about what creating multiple axes within a plot looks like in Rubyplot.

Can you also include an example which allows you to set the following basic attributes?

• Marker size
• Marker type
• Line size
• Line style
• Marker color
• Line color

Again, good work on this.

[v0dro]

@pgtgrly what are your comments on John's statement? I think he's making some excellent points.

[pgtgrly]

@v0dro @MohawkJohn I was just writing the mail about my today's progress and addressing this. I will iterate that over here first. Reading the comment I decided to instantiate a class RbPlot inside RubyPlot which acts like Figure.

I will hopefully include the example and specs for the mentioned basic attributes in the working prototype.

As for the Sub-Plots I am not sure if they can be implemented with GR Framework as this is a problem that has to dealt at the Artist and Level and GR Framework abstracts that. I will however investigate GR framework/think of a bypass after I get the prototype working. I will be mailing on the ML soon.

Edit:
Here is the changed spec that I will base the prototype on:

require 'spec_helper'

describe Rubyplot::Rbplot do
  before do
    @x1 = [-10, 0, 5, 28]
    @y1 = [1, 2, 3, 4]
    @x2 = [2, 4, 16]
    @y2 = [10, 20, -40]
  end

  context '#line #scatter' do
    it 'creates a line and scatter graph' do
      a = Rubyplot::Rbplot.new
      a.title 'My cool graph'
      a.line @x1, @y1
      a.marker_color 'cyan'
      a.scatter @x2, @y2
      a.save 'file_name.bmp'

      expect(equal_files('file_name.bmp', 'line_scatter_graph.bmp')).to eq(true)
    end
  end
end

[translunar]

Okay, but why call it RbPlot? This is confusing. Why not just call it Figure? Metaphors are really important, and people already understand Figure because they use Figure in Matplotlib and Matlab.

I'd also suggest you spend some time thinking about subplots now. Every Figure has at least one set of Axes. Users should be able to easily add additional Axes. Actually, from what you've got in your code, it looks like Rbplot is more akin to Matplotlib's Axes than it is to Matplotlib's Figure class.

[arafatkatze]

@pgtgrly @v0dro Thanks
This helped a lot.

[translunar]

What thinking have you done regarding my comment from yesterday?

[arafatkatze]

Hi John,
Are you referring to me?

dad4455

[translunar]

I am referring to you. But I was actually asking about this comment: #4 (comment)

In addition, it's best practice — every time you do a commit that relates to an issue — to tag the issue number in the commit comment.

So your comment should be something like, "Adding Test mechanism to check image diffs against reference images. WIP #4." WIP stands for work in progress. When something fixes an issue, do "Fixes #4," and it'll actually automatically close the issue for you.

Now, with regards to dad4455, this is an overly complicated solution. Have you looked at the function matplotlib uses for all of its unit tests?

Here's the link:
https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/testing/compare.py#L366

Why don't I like your solution? You're adding an additional gem requirement to a project. That makes everything an order of magnitude more complicated. You're occasionally going to need some additional gems (e.g., for magick), and that's unavoidable. But you should do everything you can to avoid introducing more complexity.

The meat of the method above is here:
https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/testing/compare.py#L431

It's not hard to compute the RMS of two images. RMS is just the root-mean-square. DM me if you have trouble understanding the math.

[arafatkatze]

All the potential extra plots to be added in the future are described here and the specs are in this branch.