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

[BREAKING] Return 0 missing count for columns of nonmissing type #2360

Merged
merged 3 commits into from
Aug 13, 2020

Conversation

nilshg
Copy link
Contributor

@nilshg nilshg commented Aug 11, 2020

This came up on Slack - as the eltype is printed in the returned table, there's little additional information in returning nothing, as the user can see from eltype whether the columns allows missing or not. Returning 0 here makes sorting the table on this column easier.

.vscode/settings.json Outdated Show resolved Hide resolved
Copy link
Member

@bkamins bkamins left a comment

Choose a reason for hiding this comment

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

Looks good apart for the new file that should not land here. In the past the distinction was required, but recently we started to show full eltype (rather than stripped one as we did in the past).

Added by accident - thanks VSCode!!!
@bkamins bkamins changed the title Return 0 missing count for columns of nonmissing type [BREAKING] Return 0 missing count for columns of nonmissing type Aug 11, 2020
@bkamins bkamins added breaking The proposed change is breaking. feature labels Aug 11, 2020
@bkamins bkamins added this to the 1.0 milestone Aug 11, 2020
@bkamins
Copy link
Member

bkamins commented Aug 11, 2020

As a general note:

  1. I would like to highlight that we now avoid making breaking changes. Also for future readers - please make sure to add [BREAKING] in PR name if it is breaking
  2. In this case I think it is ok to be breaking, as it is mildly breaking and should not actually break any production workflow (unless someone relied on the old behaviour, but because of this change I will not backport it)
  3. Still I will wait for @nalimilan to approve before merging, as it is breaking

Thank you for the PR.

@bkamins bkamins merged commit 07c3bc9 into JuliaData:master Aug 13, 2020
@bkamins
Copy link
Member

bkamins commented Aug 13, 2020

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking The proposed change is breaking. feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants