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

[DOC] Implement dataset module and add new documentation #101

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

felipeffm
Copy link
Contributor

Discuss:

how make documentation more readable for all:
Sugestion: separe advanced concepts and make it visual

how to simplify the code at examples.
Sugestion: preprocess outside code

Copy link

codecov bot commented Aug 5, 2024

Codecov Report

Attention: Patch coverage is 76.19048% with 15 lines in your changes missing coverage. Please review.

Project coverage is 93.91%. Comparing base (e1683e3) to head (10878dc).

Files with missing lines Patch % Lines
...c/prophetverse/examples/repository/repositories.py 67.74% 10 Missing ⚠️
src/prophetverse/examples/repository/base.py 76.47% 4 Missing ⚠️
src/prophetverse/examples/preprocess_datasets.py 91.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #101      +/-   ##
==========================================
- Coverage   94.85%   93.91%   -0.95%     
==========================================
  Files          26       30       +4     
  Lines        1186     1249      +63     
==========================================
+ Hits         1125     1173      +48     
- Misses         61       76      +15     

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

@felipeangelimvieira felipeangelimvieira changed the title Docs/new structure [DOC] Implement dataset module and add new documentation Aug 11, 2024
@felipeangelimvieira
Copy link
Owner

Related issue #102

@felipeangelimvieira
Copy link
Owner

@felipeffm as we've talked today, I think it would be nice to have a structure similar to sktime, with one function for each dataset
I liked the idea of having a file with common plotting functions used in the documentation examples as you are implementing. Thank you very much for proposing this PR :)

@fkiraly
Copy link
Contributor

fkiraly commented Sep 13, 2024

might this also be relevant?
sktime/sktime#4333

@felipeangelimvieira
Copy link
Owner

might this also be relevant? sktime/sktime#4333

yes! Felipe told me he won't be able to finish this PR due to bandwidth limitation, I'm planning to implement it inheriting BaseObject from skbase - now potentially BaseDataset from sktime. I can imagine that this sktime's PR still need some work, and all datasets would need to be adapted, isn't it?

@fkiraly
Copy link
Contributor

fkiraly commented Sep 21, 2024

I can imagine that this sktime's PR still need some work, and all datasets would need to be adapted, isn't it?

Yes, that's right - it is just a design study at the moment, not even a fully developed API. It was created for the benefit of some contributors who were considering to work on this but then abandoned the project.

So, if you create an API in prophetverse and it looks good and adaptable, I would very much also like to adapt it for sktime. Or, if you even implement such an interface in sktime first, and then "inherit" it in prophetverse, that would be even nicer! Either would be appreciated, as I also do not have bandwidth for the data sets module at the moment.

@felipeangelimvieira
Copy link
Owner

Alright, I'll implement it in sktime and then use it in Prophetverse. I'm currently on vacation and should start working on that in about 2 weeks.

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