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

ENH: add method='dense' to rank #6514

Closed
wants to merge 3 commits into from
Closed

ENH: add method='dense' to rank #6514

wants to merge 3 commits into from

Conversation

dsm054
Copy link
Contributor

@dsm054 dsm054 commented Mar 1, 2014

Addresses #6333.

(Let's see how it goes on Travis; last time there was some strange behaviour which I think turned out to be because of an uninitialized variable which was behaving differently on my system than on the buildbots.)

@jreback
Copy link
Contributor

jreback commented Mar 1, 2014

looks good
pls add a release note entry (and same in v0.14 if u want)

then good. 2 go

@jreback
Copy link
Contributor

jreback commented Mar 1, 2014

are their frame tests at all?

@dsm054
Copy link
Contributor Author

dsm054 commented Mar 1, 2014

Yeah, test_rank_2d_tie_methods loops over every rank method and checks the frame behaviour. Useful, too-- one of my early versions was resetting the count at the wrong place, and so it was ranking the whole flattened frame rather than each column.

@jreback
Copy link
Contributor

jreback commented Mar 1, 2014

ahh ok makes sense

@jreback
Copy link
Contributor

jreback commented Mar 1, 2014

perfect pls just add release notes and can merge

@dsm054
Copy link
Contributor Author

dsm054 commented Mar 1, 2014

So does this go under "Improvements to existing features" for 0.14.0?

@jreback
Copy link
Contributor

jreback commented Mar 1, 2014

that works

@dsm054
Copy link
Contributor Author

dsm054 commented Mar 1, 2014

It is my considered opinion that git is sufficiently complicated that having a tutorial reference open while using it is useless, because there is always some state I'm in which prevents the command from working as it's supposed to. And this is a fact which beginners have no way to know until they've done it and made things worse.

Right now the last the last three log events in my local branch add_dense_rank_v5 are (1) the original commit, (2) 7ec7662's commit, and (3) a merge which was introduced when trying to follow instructions to avoid the "Updates were rejected because a pushed branch tip is behind its remote" error.

@jreback
Copy link
Contributor

jreback commented Mar 1, 2014

hmmm...I avoid merging (except when actually merging other people commits)...

my workflow is something like this:

update master

git checkout master
git pull --rebase
git checkout branch
git pull --rebase

fix conflicts (if needed)

git add --all
git rebase --continue

add commits

rebase when needed to reorder/squash

git rebase -i <commit before this branch>
git push myfork thisbranch -f

@jreback
Copy link
Contributor

jreback commented Mar 1, 2014

you can just delete that merge commit FYI (I am going to remove it when I merge you in a moment anyhow)

@jreback
Copy link
Contributor

jreback commented Mar 1, 2014

merged via 61b141b

@jreback
Copy link
Contributor

jreback commented Mar 1, 2014

thanks as always!

@dsm054 dsm054 deleted the add_dense_rank_v5 branch March 1, 2014 15:46
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