[DOC] General PR to improve docs

[julian-fong]

Description

This PR aims to improve current docstring and revamp the contribute/installation page for users looking to get working copies of pytorch-forecasting or would like to get a developmental version for contributing.

Checklist

• Linked issues (if existing)
  [DOC] review of developer documentation #1635 and [DOC] Request for Recommended Environment Setup for Temporal Fusion Transformer Tutorial #1698
• Amended changelog for large changes (and added myself there as contributor)
• Added/modified tests
• Used pre-commit hooks when committing to ensure that code is compliant with hooks. Install hooks with pre-commit install.
  To run hooks independent of commit, execute pre-commit run --all-files

[julian-fong]

@fkiraly @yarnabrina reviews appreciated if possible :)

[yarnabrina]

This is an incomplete review. I think the last part of changes are from the deleted file, any reason to not have it as it is? In a lot of Github repositories, CONTRIBUTING.md or similar seems quite standard.

[yarnabrina]

I think it needs a new line before this. Please verify.

[julian-fong]

This is an incomplete review. I think the last part of changes are from the deleted file, any reason to not have it as it is? In a lot of Github repositories, CONTRIBUTING.md or similar seems quite standard.

I've opted to replace the contributing.md with installation.md and include the contributing section inside the installation.md file. This was also done to match sktime and skpro, do you have a preference otherwise? I suppose they could be split into two separate files, I'm good with either way

[fkiraly]

is this ready for review, @julian-fong?

[julian-fong]

is this ready for review, @julian-fong?

This is ready for review.

[yarnabrina]

@fkiraly / @julian-fong do we have the read the docs build per pull request now? I'd like to see whether it renders correctly.

[fkiraly]

@yarnabrina, no, we do not have this yet.

I opened an issue to track this as a maintenance/CI work item: #1708

Do you know how to enable this? If yes, kindly add instructions or links in #1708.

[yarnabrina]

Added few comments. Since RTD is enabled now, next commit should help us to see the docs easily.

[yarnabrina]

This line looks odd. Did you miss a command, or did you intend it to be part if point 2?

[julian-fong]

I think everything is intended here, I copied it off of the sktime installation instructions - see https://www.sktime.net/en/stable/installation.html#full-developer-setup-for-contributors-and-extension-developers

[yarnabrina]

To be precise what looked odd to me, that is this section. It seems bullets 2-4 are under bullet 1, and bullet 5 is once again at same level as bullet 1.

I am not formally requesting a change, but this did look weird to me personally, both here and in sktime too (which I definitely missed earlier).

[yarnabrina]

Question: does is need to be a separate install?

[julian-fong]

It does not need to be a separate install since it is one of the pytorch-forecasting core dependencies. I thought it would be a good idea to include a step to install the package since it is such an important library (similar to torch).

We can remove it if its not needed

[yarnabrina]

I am not sure if the path for personal forks is the most common path. Won't it be <username>/pytorch-forecasting instead of <username>/sktime/pytorch-forecasting by default?

[julian-fong]

I'm not 100% sure to be honest @fkiraly do you know if this is the correct path?

[julian-fong]

I suppose it would be a good idea to have a couple users try and follow the instructions to see if everything works properly or not

[julian-fong]

@yarnabrina how does it look? I see that on the page there is an empty 'section navigation' on the left side - not exactly sure how to remove that

[yarnabrina]

@yarnabrina how does it look? I see that on the page there is an empty 'section navigation' on the left side - not exactly sure how to remove that

My guess is in the rst itself, not sure. I think specifying the initial ".. " as we have in sktime may have something to do with it, but it's a blind guess and not tested.