Skip to content

Add installation instruction #526

Merged
merged 7 commits into from
Feb 16, 2022
Merged

Add installation instruction #526

merged 7 commits into from
Feb 16, 2022

Conversation

Mr-Geekman
Copy link
Contributor

@Mr-Geekman Mr-Geekman commented Feb 11, 2022

IMPORTANT: Please do not create a Pull Request without creating an issue first.

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?

Type of Change

  • Examples / docs / tutorials / contributors update
  • Bug fix (non-breaking change which fixes an issue)
  • Improvement (non-breaking change which improves an existing feature)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Proposed Changes

Look #490.

Related Issue

#490.

Closing issues

Closes #490.

@Mr-Geekman Mr-Geekman self-assigned this Feb 11, 2022
@Mr-Geekman Mr-Geekman added the documentation Improvements or additions to documentation label Feb 11, 2022
@codecov-commenter
Copy link

codecov-commenter commented Feb 11, 2022

Codecov Report

Merging #526 (0892c1c) into master (a7a08c8) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #526   +/-   ##
=======================================
  Coverage   88.21%   88.21%           
=======================================
  Files         118      118           
  Lines        5677     5677           
=======================================
  Hits         5008     5008           
  Misses        669      669           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7a08c8...0892c1c. Read the comment docs.

Copy link
Contributor

@iKintosh iKintosh left a comment

Choose a reason for hiding this comment

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

Added some comments. Let's discuss it 🙂

README.md Outdated
```bash
pip install --upgrade pip
pip install etna
```

Default version hasn't all the dependencies, because some of them are needed only for specific models, e.g. Prophet, PyTorch.
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to say The default version does not contain all the dependencies

README.md Outdated
```bash
pip install --upgrade pip
pip install etna
```

Default version hasn't all the dependencies, because some of them are needed only for specific models, e.g. Prophet, PyTorch.
Available extensions are listed at [`pyproject.toml`](https://github.com/tinkoff-ai/etna/blob/master/pyproject.toml#L93).
Copy link
Contributor

Choose a reason for hiding this comment

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

It's better to list it right here.

Available extensions are the following: ...

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 think it can be difficult to support it after adding new extensions or changing current.

Comment on lines +63 to +66
Install extension:
```bash
pip install etna[extension-name]
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably it is better to remove this part, as the all available extensions will be already listed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even if extensions are listed it can be not obvious how to install them. This instruction makes it clear.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe list extensions as installation instructions:

pip install etna[prophet]
pip install etna[torch]
pip install etna[wandb]

Copy link
Contributor

Choose a reason for hiding this comment

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

If you are against it, let's keep it your way.

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 think that we should just move information about how to install extensions higher.

README.md Outdated
Comment on lines 74 to 76

It can be useful to configure the library to check installed packages during init.
For example, if you know that you need `Prophet` in you project and don't want to understand that it isn't installed only during the import of `etna.models.ProphetModel`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this way it will clearer:

etna supports configuration files. It means that etna will check that all the specified packages are installed prior to script start and NOT during runtime.

README.md Outdated
It can be useful to configure the library to check installed packages during init.
For example, if you know that you need `Prophet` in you project and don't want to understand that it isn't installed only during the import of `etna.models.ProphetModel`.

You can create a config file `.etna` at the root of your project, where this checking can be set up. To see the available options look at [`Settings`](https://github.com/tinkoff-ai/etna/blob/master/etna/settings.py#L68). There is an [example](https://github.com/tinkoff-ai/etna/tree/master/examples/configs/.etna) of configuration file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
You can create a config file `.etna` at the root of your project, where this checking can be set up. To see the available options look at [`Settings`](https://github.com/tinkoff-ai/etna/blob/master/etna/settings.py#L68). There is an [example](https://github.com/tinkoff-ai/etna/tree/master/examples/configs/.etna) of configuration file.
To create a config file you should create `.etna` file at the root of your project. To see the available options look at [`Settings`](https://github.com/tinkoff-ai/etna/blob/master/etna/settings.py#L68). There is an [example](https://github.com/tinkoff-ai/etna/tree/master/examples/configs/.etna) of configuration file.

iKintosh
iKintosh previously approved these changes Feb 16, 2022
Copy link
Contributor

@iKintosh iKintosh left a comment

Choose a reason for hiding this comment

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

Looks good to me!

README.md Outdated
* `torch`
* `wandb`

There are also developer extensions. All the extensions are listed at [`pyproject.toml`](https://github.com/tinkoff-ai/etna/blob/master/pyproject.toml#L93).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
There are also developer extensions. All the extensions are listed at [`pyproject.toml`](https://github.com/tinkoff-ai/etna/blob/master/pyproject.toml#L93).
There are also developer extensions. All the extensions are listed in [`pyproject.toml`](https://github.com/tinkoff-ai/etna/blob/master/pyproject.toml#L93).

Comment on lines +63 to +66
Install extension:
```bash
pip install etna[extension-name]
```
Copy link
Contributor

Choose a reason for hiding this comment

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

If you are against it, let's keep it your way.

Copy link
Contributor

@iKintosh iKintosh left a comment

Choose a reason for hiding this comment

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

🔥

@Mr-Geekman Mr-Geekman merged commit 27acb99 into master Feb 16, 2022
@Mr-Geekman Mr-Geekman deleted the issue-490 branch February 16, 2022 14:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

README: update installation
3 participants