Skip to content

Fix BaseReconciliator to work on pandas==1.1.5 #1229

Merged
merged 3 commits into from
Apr 18, 2023
Merged

Conversation

Mr-Geekman
Copy link
Contributor

@Mr-Geekman Mr-Geekman commented Apr 17, 2023

Before submitting (must do checklist)

  • Did you read the contribution guide?
  • Did you update the docs? We use Numpy format for all the methods and classes.
  • Did you write any new necessary tests?
  • Did you update the CHANGELOG?

Proposed Changes

Add check on length before running DataFrame.drop.

Closing issues

Closes #1212.

@Mr-Geekman Mr-Geekman self-assigned this Apr 17, 2023
@@ -815,7 +815,7 @@ def to_hierarchical_dataset(
"""
df_copy = df.copy(deep=True)
df_copy["segment"] = df_copy[level_columns].astype("string").agg(sep.join, axis=1)
if not keep_level_columns:
if not keep_level_columns and len(level_columns) > 0:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not really sure if we should do this check with len here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

In fact we don't as this method expect to exist at least one column in level_columns

Copy link
Collaborator

Choose a reason for hiding this comment

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

May be we can throw error in the beginning if it is empty

@github-actions
Copy link

github-actions bot commented Apr 17, 2023

@codecov-commenter
Copy link

codecov-commenter commented Apr 17, 2023

Codecov Report

Merging #1229 (468774b) into master (8168c53) will increase coverage by 0.21%.
The diff coverage is 100.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #1229      +/-   ##
==========================================
+ Coverage   87.74%   87.95%   +0.21%     
==========================================
  Files         186      186              
  Lines       10614    10621       +7     
==========================================
+ Hits         9313     9342      +29     
+ Misses       1301     1279      -22     
Impacted Files Coverage Δ
etna/datasets/tsdataset.py 92.87% <100.00%> (+0.02%) ⬆️
etna/reconciliation/base.py 94.59% <100.00%> (+0.15%) ⬆️

... and 6 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions github-actions bot temporarily deployed to pull request April 17, 2023 12:03 Inactive
@@ -815,7 +815,7 @@ def to_hierarchical_dataset(
"""
df_copy = df.copy(deep=True)
df_copy["segment"] = df_copy[level_columns].astype("string").agg(sep.join, axis=1)
if not keep_level_columns:
if not keep_level_columns and len(level_columns) > 0:
Copy link
Collaborator

Choose a reason for hiding this comment

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

In fact we don't as this method expect to exist at least one column in level_columns

@@ -815,7 +815,7 @@ def to_hierarchical_dataset(
"""
df_copy = df.copy(deep=True)
df_copy["segment"] = df_copy[level_columns].astype("string").agg(sep.join, axis=1)
if not keep_level_columns:
if not keep_level_columns and len(level_columns) > 0:
Copy link
Collaborator

Choose a reason for hiding this comment

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

May be we can throw error in the beginning if it is empty

@github-actions github-actions bot temporarily deployed to pull request April 18, 2023 10:18 Inactive
@alex-hse-repository alex-hse-repository merged commit 6eee08e into master Apr 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Failed tests for hierarchical pipeline on pandas==1.1.5
3 participants