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

Automatic Binning #131

Closed
wants to merge 12 commits into from
Closed

Automatic Binning #131

wants to merge 12 commits into from

Conversation

MuellerSeb
Copy link
Member

@MuellerSeb MuellerSeb commented Jan 14, 2021

Closes: #55 #36

This PR adds a simple automatic binning routine standard_bins to the variogram submodule.
It returns an equal division from 0 to max_dist with bin_no as number of bins. The following rules are applied:

  • if max_dist is not given, it is set to one third of the box diameter
  • if bin_no is not given, it is determined by sturges' rule from the number of point combinations

You can now call gs.vario_estimate without passing bin_edges. In this case, the bin_edges will be determined as described above.

@MuellerSeb MuellerSeb added the enhancement New feature or request label Jan 14, 2021
@MuellerSeb MuellerSeb added this to the 1.3 milestone Jan 14, 2021
@MuellerSeb MuellerSeb requested a review from LSchueler January 14, 2021 12:07
@MuellerSeb MuellerSeb self-assigned this Jan 14, 2021
@MuellerSeb
Copy link
Member Author

@LSchueler ready for review. 😊

@MuellerSeb MuellerSeb linked an issue Jan 14, 2021 that may be closed by this pull request
Copy link
Member

@LSchueler LSchueler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a really nice convenience addition!

What do you think of moving the _chordal_to_great_circle method to a more visible place and making it public?

Appart from that, I think we are ready to merge this PR.

gstools/variogram/binning.py Outdated Show resolved Hide resolved
@LSchueler
Copy link
Member

I fixes some typos, so don't forget to pull.

@MuellerSeb
Copy link
Member Author

MuellerSeb commented Jan 15, 2021

Strange. It was merged, but this PR says it has conflicts and can't be merged. It felt like an internal GitHub Error when I clicked on "merge" and I had to reload the page. Hope nothing was broken.

Edit: after closing, it says that it was merged successfully.

@MuellerSeb MuellerSeb closed this Jan 15, 2021
@MuellerSeb MuellerSeb deleted the binning branch January 15, 2021 20:15
@MuellerSeb MuellerSeb mentioned this pull request Jan 15, 2021
@MuellerSeb MuellerSeb restored the binning branch January 15, 2021 20:22
@MuellerSeb MuellerSeb deleted the binning branch January 15, 2021 20:23
MuellerSeb added a commit that referenced this pull request Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Automatic binning Ordinary Kriging: help please?
2 participants