Skip to content

better meta inferrence, to prevent indexing error while parquetting - #877

Open
Apsylem wants to merge 7 commits into
blue-yonder:mainfrom
Apsylem:better-meta-inferrence-for-df.apply-in-dask_feature_extraction_on_chunk
Open

Apsylem wants to merge 7 commits into
blue-yonder:mainfrom
Apsylem:better-meta-inferrence-for-df.apply-in-dask_feature_extraction_on_chunk

Conversation

@Apsylem

@Apsylem Apsylem commented Jul 9, 2021

Copy link
Copy Markdown

a unneccary indexing over cloumns in _feature_extraction_on_chunk_helper has been removed

but mainly, an option "taste_of_pandas_df" has been addet to dask_feature_extraction_on_chunk.
This example of the input is used to infer the correct meta in df.apply. This was neccecery due to missing and misleading index values while parquetting the data after tsfresh feature extraction. No Bug report has been filed for this issue.

@codecov-commenter

codecov-commenter commented Jul 9, 2021 •

Copy link
Copy Markdown

Codecov Report

Merging #877 (b81e6bc) into main (b9a7b18) will decrease coverage by 0.14%.
The diff coverage is 63.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #877      +/-   ##
==========================================
- Coverage   95.34%   95.19%   -0.15%     
==========================================
  Files          18       18              
  Lines        1867     1872       +5     
  Branches      368      369       +1     
==========================================
+ Hits         1780     1782       +2     
- Misses         47       49       +2     
- Partials       40       41       +1     
Impacted Files Coverage Δ
tsfresh/convenience/bindings.py 53.33% <55.55%> (-2.67%) ⬇️
tsfresh/feature_extraction/extraction.py 95.06% <100.00%> (ø)
tsfresh/feature_extraction/feature_calculators.py 97.60% <100.00%> (ø)

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 b9a7b18...b81e6bc. Read the comment docs.

@nils-braun

Copy link
Copy Markdown
Collaborator

This PR also includes the changes of #874. I will wait for #874 to be finished (minus the changes in this PR) - after that we can have a look into the changes here individually.

@Apsylem

Apsylem commented Jul 12, 2021

Copy link
Copy Markdown
Author

I am sorry, this is my first pull request in a row and one of my first in general. Is there a better way to build upon my own changes, if the old pull request has not yet been accepted? I would aapreciate any remarks.

@nils-braun

Copy link
Copy Markdown
Collaborator

Cool! Then thanks for taking your time and starting with open-source development!

This is a good question and there are some options:

  • Easiest solution is to just wait for the other PR to be included before creating a new one. That sounds trivial, but is the solution I normally use.
  • If you want to open the PR anyway, open it against the older PR. We can then work on both PR in parallel, the changes of the old PR are automatically included in the new one and are still separate. In this case, you typically merge the PRs in the order "new, old". Just make sure you mention all related persons and explain them why we have two PRs.
  • You can also open the PRs as you have done but clearly state the fact that those depend on each other. I would however not recommend to do so, because it is hard to review the changes independently.

The absolut best option is to separate the changes into two independent PRs. That is not always possible though.

@Apsylem

Apsylem commented Aug 24, 2021

Copy link
Copy Markdown
Author

I am sorry, but I don't see the whitespaces, the error report on my other pull request, showed the locations to be modified.

here it says :

  • hook id: trailing-whitespace
  • exit code: 1
  • files were modified by this hook

does this mean, the trailing whitespaces have already been removed ?

@nils-braun

nils-braun commented Aug 24, 2021 •

Copy link
Copy Markdown
Collaborator

Hi @Apsylem !
You can check the full log of the automatic build result (or by clicking on "Details"). It will show you all problematic lines. The build will not edit your code (fortunately ;-)) and will therefore not fix the style errors for you.

If you have followed our installation for developers description on the docu, you should have "pre-commit" installed, which will trigger fixing these style issues locally on your computer before you push to github. If you do not want to do this, you can at least call black and isort by yourself (but using pre-commit is preferred).
If you do not know pre-commit, have a look into its docu.

This branch has not been deployed

No deployments
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.

3 participants