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

as.df removes index #5043

Merged
merged 1 commit into from
Jun 21, 2021
Merged

as.df removes index #5043

merged 1 commit into from
Jun 21, 2021

Conversation

jangorecki
Copy link
Member

@jangorecki jangorecki commented Jun 12, 2021

@codecov
Copy link

codecov bot commented Jun 12, 2021

Codecov Report

Merging #5043 (1bda2fa) into master (b074df1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #5043   +/-   ##
=======================================
  Coverage   99.47%   99.47%           
=======================================
  Files          75       75           
  Lines       14807    14808    +1     
=======================================
+ Hits        14729    14730    +1     
  Misses         78       78           
Impacted Files Coverage Δ
R/data.table.R 99.94% <100.00%> (+<0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b074df1...1bda2fa. Read the comment docs.

@mattdowle mattdowle added this to the 1.14.1 milestone Jun 21, 2021
@mattdowle
Copy link
Member

Yes completely agree with this, especially when it's seen in context just after sorted is removed too.
So will merge this PR as-is (without news item) with the issue left open in this milestone. Seems like you want to confirm this fixes it first before writing news item and closing the issue, which would need folks to test using the dev version built off main branch.

@mattdowle mattdowle merged commit 628fdee into master Jun 21, 2021
@mattdowle mattdowle deleted the rm-index branch June 21, 2021 21:48
@mattdowle
Copy link
Member

I added NEWS item because this is what's causing NMdata to fail its tests, #5075. Easy one to resolve as a PR to NMdata which I'll do and point to the news item.
I see it reported in #5042 that this PR doesn't fix the issue. It's tagged this milestone so I'll look at that again before release.

@mattdowle
Copy link
Member

Looks like PR #4893 was earlier and did setDF too. So I updated news item 27 in v1.14.2 to reflect that.

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