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

Add new function fetch_megafon #115

Merged
merged 4 commits into from
Jul 27, 2021
Merged

Conversation

ezhdi
Copy link
Contributor

@ezhdi ezhdi commented Jul 17, 2021


name: "Add new function fetch_megafon"
about: Add Megafon dataset in sklift.datasets described in issue #99

📑 Description of the Change

Add new function fetch_megafon in sklift.datasets that loads and return train part of the Megafon dataset

Verification Process

Release Notes

Additional info

@maks-sh
Copy link
Owner

maks-sh commented Jul 20, 2021

Hey!

Thank you 🙌 Amazing work 👍

There are a couple of small comments:

  1. Since we do not have the ability to select a target, it makes no sense to create the `target_col ' argument. The fetch_x5 function is made similarly.
  2. Unfortunately, now the documentation pages are not created automatically. Therefore, when adding new functions, you need to manually add a page to the desired directory. In your case, this directory is scikit-uplift/docs/api/datasets/. Please create a page along the way with content similar to fetch_x5 function.
  3. It would be great to add the Key figures section to descr/megafon.rst by analogy with the criteo dataset (Format, Size, Rows, etc.).

I apologize that these items were not described in the issue.

@ezhdi
Copy link
Contributor Author

ezhdi commented Jul 21, 2021

Hi! fix all comments and bug when import all datasets too

@ezhdi
Copy link
Contributor Author

ezhdi commented Jul 22, 2021

Add example of usages for fetch_megafon to the docstrings

@maks-sh maks-sh linked an issue Jul 26, 2021 that may be closed by this pull request
@maks-sh maks-sh merged commit b18990b into maks-sh:dev Jul 27, 2021
@maks-sh
Copy link
Owner

maks-sh commented Jul 27, 2021

Thank you for your contribution!

Please do not forget to make a submission on the ods.ai with a link to this PR.

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.

Add Megafon dataset in sklift.datasets
2 participants