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

[Suggestion] Calculate Quartile from the instance of Float64Data #60

Closed
pararang opened this issue Feb 19, 2021 · 4 comments
Closed

[Suggestion] Calculate Quartile from the instance of Float64Data #60

pararang opened this issue Feb 19, 2021 · 4 comments

Comments

@pararang
Copy link
Contributor

pararang commented Feb 19, 2021

Nice package, I am using it right now. And I found an inconsistency while calculating the quartiles. Any reason why we must pass the data/input to calculate Quartile? Why not use the instance. If there is no specific reason, I suggest adding the Quartiles method on the Float64Data's struct without any input, but use the current instance, like we use Mean(), Max(), etc.

Suggestion:

func (f Float64Data) Quartiles() (Quartiles, error) {
	return Quartile(f)
}

If this is possible, I will make the MR.

@pararang pararang changed the title Calculate Quartile from the instance of Float64Data [Suggestion] Calculate Quartile from the instance of Float64Data Feb 19, 2021
@montanaflynn
Copy link
Owner

Hello @pararang, this is a good catch, there is no reason why the data.Quartile method needs the input. The suggestion to add Quartiles() is good since it wouldn't be a breaking change. Make a pull request and I'll take a look.

@pararang
Copy link
Contributor Author

Alright, I'm working on it.

@pararang
Copy link
Contributor Author

Submitted on PR #61, Please take a look, @montanaflynn.

montanaflynn pushed a commit that referenced this issue Feb 21, 2021
* chore: git ignore vscode config workspace
* feat: method to get  quartiles without input/argument from the initiated data64float
* test: fix test for method Quartiles
@montanaflynn
Copy link
Owner

Thanks! I merged it and will issue a new release version soon.

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

No branches or pull requests

2 participants