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.

Sign up for GitHub

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

Jump to bottom

Refactor PymfeExtractor interface #83

Merged
merged 13 commits into from
Dec 12, 2023
Merged

Refactor PymfeExtractor interface #83

merged 13 commits into from
Dec 12, 2023

Conversation

MorrisNein
Copy link
Collaborator

@MorrisNein MorrisNein commented Nov 24, 2023 •
edited
Loading

Changes:

@pep8speaks
Copy link

pep8speaks commented Nov 24, 2023 •
edited
Loading

Hello @MorrisNein! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 1:1: F401 '.dataset_base.DatasetBase' imported but unused
Line 1:1: F401 '.dataset_base.DatasetData' imported but unused
Line 1:1: F401 '.dataset_base.DatasetIDType' imported but unused
Line 1:1: F401 '.dataset_base.TabularData' imported but unused
Line 1:1: F401 '.dataset_base.TimeSeriesData' imported but unused
Line 2:1: F401 '.custom_dataset.CustomDataset' imported but unused
Line 2:1: F401 '.custom_dataset.DataNotFoundError' imported but unused
Line 3:1: F401 '.openml_dataset.OpenMLDataset' imported but unused
Line 3:1: F401 '.openml_dataset.OpenMLDatasetIDType' imported but unused
Line 4:1: F401 '.time_series_dataset.TimeSeriesDataset' imported but unused

Line 1:1: F401 'meta_automl.data_preparation.file_system.file_system.PathType' imported but unused
Line 1:1: F401 'meta_automl.data_preparation.file_system.file_system.get_project_root' imported but unused
Line 1:1: F401 'meta_automl.data_preparation.file_system.file_system.get_data_dir' imported but unused
Line 2:1: F401 'meta_automl.data_preparation.file_system.cache.CacheOperator' imported but unused
Line 2:1: F401 'meta_automl.data_preparation.file_system.cache.get_cache_dir' imported but unused
Line 2:1: F401 'meta_automl.data_preparation.file_system.cache.get_dataset_cache_path' imported but unused
Line 2:1: F401 'meta_automl.data_preparation.file_system.cache.get_dataset_cache_path_by_id' imported but unused
Line 2:1: F401 'meta_automl.data_preparation.file_system.cache.update_openml_cache_dir' imported but unused

Line 4:1: F401 'pandas as pd' imported but unused

Comment last updated at 2023-12-12 18:24:44 UTC

@codecov Codecov
Copy link

codecov bot commented Nov 24, 2023 •
edited
Loading

Codecov Report

Attention: 46 lines in your changes are missing coverage. Please review.

Comparison is base (bffab03) 29.33% compared to head (2d61733) 28.42%.

❗ Current head 2d61733 differs from pull request most recent head e25ccdd. Consider uploading reports for the commit e25ccdd to get more accurate results

Files Patch % Lines
...ion/feature_preprocessors/feature_preprocessors.py 11.11% 24 Missing ⚠️
..._preparation/surrogate_dataset/dataset_generate.py 0.00% 7 Missing ⚠️
...ta_automl/data_preparation/dataset/dataset_base.py 76.19% 5 Missing ⚠️
...ration/meta_features_extractors/pymfe_extractor.py 88.88% 5 Missing ⚠️
...meta_algorithm/model_advisors/surrogate_advisor.py 0.00% 3 Missing ⚠️
.../meta_features_extractors/dataset_meta_features.py 91.66% 1 Missing ⚠️
...rity_assessors/model_based_similarity_assessors.py 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main      #83      +/-   ##
==========================================
- Coverage   29.33%   28.42%   -0.91%     
==========================================
  Files          53       54       +1     
  Lines        2359     2318      -41     
==========================================
- Hits          692      659      -33     
+ Misses       1667     1659       -8

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@MorrisNein MorrisNein requested a review from nicl-nno November 27, 2023 11:51
@MorrisNein MorrisNein force-pushed the refactor-pymfe-interface branch from d496bcb to 8f36254 Compare December 4, 2023 17:33
This was linked to issues Dec 4, 2023
Reconsider the interface of PymfeExtractor #75
Closed
Unify the input interface of MetaFeaturesExtractor #76
Closed
Clarify cache usage #77
Closed
nicl-nno
nicl-nno reviewed Dec 8, 2023
View reviewed changes
meta_automl/data_preparation/dataset/dataset_base.py
forecast_length: int = 1


class DatasetBase(ABC, CacheOperator):
DatasetDataType_co = TypeVar('DatasetDataType_co', bound=DatasetData, covariant=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

А почему _co?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

PEP рекомендуют суффиксы _co и _contra для covariant и contravariant типов соответственно

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

https://peps.python.org/pep-0484/#covariance-and-contravariance

nicl-nno
nicl-nno approved these changes Dec 8, 2023
View reviewed changes
Copy link
Contributor

@nicl-nno nicl-nno left a comment

Choose a reason for hiding this comment

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

Вроде каких-то возражений нет.

ChernyakKonstantin
ChernyakKonstantin reviewed Dec 9, 2023
View reviewed changes
meta_automl/data_preparation/feature_preprocessors/feature_preprocessors.py Outdated
@@ -7,6 +7,24 @@
from sklearn.preprocessing import StandardScaler


def explode_ungrouped_metafeatures(df: pd.DataFrame, n_features: int) -> pd.DataFrame:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Добавить описание, для чего этот метод.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Добавил

@MorrisNein MorrisNein force-pushed the refactor-pymfe-interface branch from 8f36254 to 2d61733 Compare December 12, 2023 18:24
@MorrisNein MorrisNein merged commit ce147ff into main Dec 12, 2023
1 check passed
@MorrisNein MorrisNein deleted the refactor-pymfe-interface branch December 12, 2023 18:41
@MorrisNein MorrisNein mentioned this pull request Dec 13, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChernyakKonstantin ChernyakKonstantin ChernyakKonstantin left review comments

@nicl-nno nicl-nno nicl-nno approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@MorrisNein @pep8speaks @nicl-nno @ChernyakKonstantin