Skip to content
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

add histogram module #354

Merged
merged 6 commits into from
Oct 8, 2018
Merged

add histogram module #354

merged 6 commits into from
Oct 8, 2018

Conversation

tech4GT
Copy link
Member

@tech4GT tech4GT commented Sep 29, 2018

Signed-off-by: tech4GT [email protected]
@jywarren Currently this generates a 256x256 histogram, I have not used a plotting library because of their slower performance.
Sample
image1_1
Issue #69

Signed-off-by: tech4GT <[email protected]>
@ghost ghost assigned tech4GT Sep 29, 2018
@ghost ghost added the in-progress label Sep 29, 2018
@tech4GT tech4GT mentioned this pull request Sep 29, 2018
@tech4GT tech4GT requested a review from jywarren October 1, 2018 01:39
@tech4GT
Copy link
Member Author

tech4GT commented Oct 1, 2018

@jywarren Please take a look at this one as well, does the histogram look okay to you? It's not very pretty but almost all plotting libraries are kind of slow.

@tech4GT
Copy link
Member Author

tech4GT commented Oct 2, 2018

@jywarren Do the visuals look okay to you? Or should we use some plotting library?

@tech4GT tech4GT closed this Oct 2, 2018
@tech4GT tech4GT reopened this Oct 2, 2018
@ghost ghost added in-progress and removed in-progress labels Oct 2, 2018
@jywarren
Copy link
Member

jywarren commented Oct 2, 2018

Wow, this is cool! What's the gradient at bottom? Perhaps we should have it generate a black graph, which can be changed in color with another module.

Thanks, Varun!

@tech4GT
Copy link
Member Author

tech4GT commented Oct 3, 2018

Hmm, I don't get you exactly, I added the gradient just to make it easier to visualize. Can you please elaborate?

@jywarren
Copy link
Member

jywarren commented Oct 3, 2018

Hi, oh i get it -- the gradient is like the x-axis label. OK i feel silly not getting that.

Can we specify the height and width of the generated histogram? Maybe that could be a follow-up issue.

I was thinking the color of the graph could be black instead of green... just because the green is nice but it's a little arbitrary. Do you think we could make that change and then people can colorize this if they want.

Another follow-up issue is that we could have the gradient toggle-able using a boolean parameter!

Great work, Varun!

@tech4GT
Copy link
Member Author

tech4GT commented Oct 4, 2018

@jywarren I forgot we already have a colorbar, so this gradient can always be added using the greyscale colorbar so maybe we should not do it here, what say?

Signed-off-by: Varun Gupta <[email protected]>
Signed-off-by: Varun Gupta <[email protected]>
Signed-off-by: Varun Gupta <[email protected]>
@tech4GT
Copy link
Member Author

tech4GT commented Oct 6, 2018

@jywarren Good to go?

Signed-off-by: Varun Gupta <[email protected]>
@tech4GT
Copy link
Member Author

tech4GT commented Oct 6, 2018

@jywarren Also if we make the histogram black, it would make the gradient indistinguishable from the actual data, so should I change the color pf the gradient bar?

@jywarren
Copy link
Member

jywarren commented Oct 8, 2018

Sorry yeah I think this is good to go. If you'd like maybe we can change the graph from green to a pure red or blue so it's simply 100% in one channel. But either way, I'll merge this if you're ready!!

@tech4GT
Copy link
Member Author

tech4GT commented Oct 8, 2018

Hmm, @jywarren don't you think having pure blue or red is a little displeasing to the eye? I mean we can always colormap this to greyscale anyway..

@jywarren
Copy link
Member

jywarren commented Oct 8, 2018 via email

@tech4GT
Copy link
Member Author

tech4GT commented Oct 8, 2018

Thanks!

@jywarren jywarren merged commit 276a637 into publiclab:main Oct 8, 2018
@ghost ghost removed the in-progress label Oct 8, 2018
@jywarren
Copy link
Member

jywarren commented Oct 8, 2018

👍👍👍 Thanks, Varun!!!

jywarren pushed a commit that referenced this pull request Oct 10, 2018
* added gif feature

* increased frame duration n changed gif btn name

* removed inline css

* improved ui

* #363 Add placeholder option to input (#370)

* Add placeholder option to input , change type of brightness input to number

 Changes to be committed:
	modified:   examples/lib/defaultHtmlStepUi.js
	modified:   src/modules/Brightness/info.json

* change 0% to 0 in brightness placeholder

* Add gamma module (#374)

* Add gamma module

* update description

* Add matrix math module (#358)

* Add matrix math module

* add info.json file

* correct format of module

* Add a constant factor input field

* clone the pixels array

* change default values

* add extra information on convolution module (#381)

* adds default values to input fields for all module steps ( fixes #382 ) (#384)

* added default values to input fields for all module steps

* merged

Signed-off-by: Ankit Singla <[email protected]>

* Accept type images in input field #364 (#366)

* gif button name changed

* fixes #383 (#396)

Signed-off-by: Varun Gupta <[email protected]>

* fix (#387)

Signed-off-by: Varun Gupta <[email protected]>

* updated

* Issue #392 Fixes alignment of message in Add step box (#393)

*  Fixes alignment of message in Add step box

* fixes css info id

* updates class selector in demo

* Spacing changes

* correctly regulates the behaviour of all links (#397)

Signed-off-by: Ankit Singla <[email protected]>

* enables Save button for an action if any of its input fields gain focus (#394)

* Updated index.html (#379)

* add histogram module (#354)

* add histogram module

Signed-off-by: tech4GT <[email protected]>

* add option to drop gradient

Signed-off-by: Varun Gupta <[email protected]>

* fix bug

Signed-off-by: Varun Gupta <[email protected]>
@jywarren jywarren mentioned this pull request Oct 21, 2018
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