Skip to content

Rework library introduction #1343

Merged
merged 12 commits into from
Aug 9, 2023
Merged

Rework library introduction #1343

merged 12 commits into from
Aug 9, 2023

Conversation

Mr-Geekman
Copy link
Contributor

@Mr-Geekman Mr-Geekman commented Aug 2, 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

Look #1334.

Closing issues

Closes #1334.

@Mr-Geekman Mr-Geekman self-assigned this Aug 2, 2023
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@codecov-commenter
Copy link

codecov-commenter commented Aug 2, 2023

Codecov Report

❗ No coverage uploaded for pull request base (docs-prototype@a35f8f2). Click here to learn what that means.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@                Coverage Diff                @@
##             docs-prototype    #1343   +/-   ##
=================================================
  Coverage                  ?   88.66%           
=================================================
  Files                     ?      193           
  Lines                     ?    12328           
  Branches                  ?        0           
=================================================
  Hits                      ?    10931           
  Misses                    ?     1397           
  Partials                  ?        0           

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

@github-actions
Copy link

github-actions bot commented Aug 2, 2023

@github-actions github-actions bot temporarily deployed to pull request August 2, 2023 12:22 Inactive
@review-notebook-app
Copy link

review-notebook-app bot commented Aug 4, 2023

View / edit / reply to this conversation on ReviewNB

alex-hse-repository commented on 2023-08-04T05:52:15Z
----------------------------------------------------------------

Looks cool!


@review-notebook-app
Copy link

review-notebook-app bot commented Aug 4, 2023

View / edit / reply to this conversation on ReviewNB

alex-hse-repository commented on 2023-08-04T05:52:16Z
----------------------------------------------------------------

Let's place it in the beginning


Mr-Geekman commented on 2023-08-04T06:42:36Z
----------------------------------------------------------------

If we place it in the beginnig, we will lose the span: "Oups. Let's fix that by looking at the table of offsets in pandas"

@review-notebook-app
Copy link

review-notebook-app bot commented Aug 4, 2023

View / edit / reply to this conversation on ReviewNB

alex-hse-repository commented on 2023-08-04T05:52:17Z
----------------------------------------------------------------

Let's skip it


@review-notebook-app
Copy link

review-notebook-app bot commented Aug 4, 2023

View / edit / reply to this conversation on ReviewNB

alex-hse-repository commented on 2023-08-04T05:52:18Z
----------------------------------------------------------------

Why do we actually have warning as we set to ignore them in the beginning?


Mr-Geekman commented on 2023-08-04T07:54:32Z
----------------------------------------------------------------

Good point, I'll try to rerun it taking this into account

@review-notebook-app
Copy link

review-notebook-app bot commented Aug 4, 2023

View / edit / reply to this conversation on ReviewNB

alex-hse-repository commented on 2023-08-04T05:52:19Z
----------------------------------------------------------------

Do we want to add some info about how transforms works? For example that the work inplace?


Mr-Geekman commented on 2023-08-04T07:55:29Z
----------------------------------------------------------------

Isn't explanation below enough?

@review-notebook-app
Copy link

review-notebook-app bot commented Aug 4, 2023

View / edit / reply to this conversation on ReviewNB

alex-hse-repository commented on 2023-08-04T05:52:20Z
----------------------------------------------------------------

Line #2.    forecast_ts.to_pandas()

Why not just forecast_ts?


@review-notebook-app
Copy link

review-notebook-app bot commented Aug 4, 2023

View / edit / reply to this conversation on ReviewNB

alex-hse-repository commented on 2023-08-04T05:52:21Z
----------------------------------------------------------------

We can end up saying that Pipeline do all this stuff inside and may be add the pseudo cod of it(optionally)


@@ -0,0 +1,230 @@
{
Copy link
Collaborator

@alex-hse-repository alex-hse-repository Aug 4, 2023

Choose a reason for hiding this comment

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

May be add link


Reply via ReviewNB

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 don't really know how to do it properly. Because we could have multiple verision of the library in documentation and we should select here only one link to one particular version.

I tried to use this: https://myst-parser.readthedocs.io/en/latest/syntax/cross-referencing.html#reference-roles , but it doesn't work with notebooks converted by nbsphinx. Probably, MyST-nb could work with this, but it requires changing the current extension for compiling notebooks.

I've found that: https://docs.readthedocs.io/en/stable/guides/jupyter.html

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure it is critical

@alex-hse-repository
Copy link
Collaborator

Problems with rendering pictures in the notebooks
Снимок экрана 2023-08-04 в 08 56 04

@alex-hse-repository
Copy link
Collaborator

Forget to add "Mechanics of forecasting" notebook
Снимок экрана 2023-08-04 в 08 57 22

README.md Outdated Show resolved Hide resolved
Copy link
Contributor Author

If we place it in the beginnig, we will lose the span: "Oups. Let's fix that by looking at the table of offsets in pandas"


View entire conversation on ReviewNB

Copy link
Contributor Author

Good point, I'll try to rerun it taking this into account


View entire conversation on ReviewNB

Copy link
Contributor Author

Isn't explanation below enough?


View entire conversation on ReviewNB

@github-actions github-actions bot temporarily deployed to pull request August 4, 2023 08:35 Inactive
@alex-hse-repository
Copy link
Collaborator

Issues with rendering:
Снимок экрана 2023-08-08 в 16 25 16

@github-actions github-actions bot temporarily deployed to pull request August 8, 2023 15:09 Inactive
@alex-hse-repository alex-hse-repository merged commit 8670ae2 into docs-prototype Aug 9, 2023
12 checks passed
@Mr-Geekman Mr-Geekman deleted the issue-1334 branch August 9, 2023 06:26
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.

3 participants