Skip to content

Enhance TSDataset to work with hierarchical series #1048

Merged
merged 18 commits into from
Dec 22, 2022

Conversation

alex-hse-repository
Copy link
Collaborator

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

Closing issues

closes #1028

@github-actions
Copy link

github-actions bot commented Dec 21, 2022

@github-actions github-actions bot temporarily deployed to pull request December 21, 2022 12:48 Inactive
@codecov-commenter
Copy link

codecov-commenter commented Dec 21, 2022

Codecov Report

❗ No coverage uploaded for pull request base (hierarchical_pipeline@62af639). Click here to learn what that means.
The diff coverage is n/a.

@@                   Coverage Diff                    @@
##             hierarchical_pipeline    #1048   +/-   ##
========================================================
  Coverage                         ?   50.96%           
========================================================
  Files                            ?      164           
  Lines                            ?     8779           
  Branches                         ?        0           
========================================================
  Hits                             ?     4474           
  Misses                           ?     4305           
  Partials                         ?        0           

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

@brsnw250
Copy link
Collaborator

Is there a dedicated method to check if particular TSDataset instance initialized with hierarchical structure?


df_level = segment_levels.pop()
level_segments = self.hierarchical_structure.get_level_segments(level_name=df_level)
if len(df_segments) != len(level_segments):
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if this check should be here or not.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Why do you think so? You mean that it is not the real case?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really think that it should be in exactly this function. Check is useful, but it looks somewhat artificial in this function because we have already got that we wanted here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, my problem with this function is what it checks this conditions every run. But according to the name it should just return the value and don't check correctness during each run.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm generally ok with this, it is optional to fix.

@alex-hse-repository
Copy link
Collaborator Author

Is there a dedicated method to check if particular TSDataset instance initialized with hierarchical structure?

You mean check smth like ts.hierarchical_structure == hierarchical_structure?

@github-actions github-actions bot temporarily deployed to pull request December 22, 2022 03:10 Inactive
@brsnw250
Copy link
Collaborator

Is there a dedicated method to check if particular TSDataset instance initialized with hierarchical structure?

You mean check smth like ts.hierarchical_structure == hierarchical_structure?

I mean how do we properly check if an instance has a hierarchical structure.? Is it ts.hierarchical_structure is None?

@github-actions github-actions bot temporarily deployed to pull request December 22, 2022 10:22 Inactive
@github-actions github-actions bot temporarily deployed to pull request December 22, 2022 10:38 Inactive
@alex-hse-repository alex-hse-repository merged commit 37ab295 into hierarchical_pipeline Dec 22, 2022
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.

4 participants