Adding `settings.ini` to PyPI source #24

sugatoray · 2022-02-23T05:26:36Z

The setup.py file reads in the config values from settings.ini. So, absence of settings.ini file in the source distribution (*.tar.gz file), leads to failure in installation of statsforecast.

Currently (v0.3.0) the PyPI source does not include the settings.ini file. This PR fixes that.
~~Changes to README.md file~~:
- ~~Fixed some of the formatting errors~~.
- ~~Fixed some broken URLs~~.
  
  The relative URLs do not render properly on PyPI. Converted them from relative to absolute URLs.

Closes #24

sugatoray · 2022-02-23T05:30:44Z

cc: @FedericoGarza

jmoralez · 2022-02-23T15:58:39Z

Thank you for your contribution! We use nbdev to develop the library, so to update the README.md file you have to modify the nbs/index.ipynb notebook and use relative links to fix the broken ones. Then run the notebook, save it and run nbdev_build_docs --fname nbs/index.ipynb from the terminal to update the README.md file. Let us know if you need any help.

sugatoray · 2022-02-23T18:53:44Z

Thank you for your contribution! We use nbdev to develop the library, so to update the README.md file you have to modify the nbs/index.ipynb notebook and use relative links to fix the broken ones. Then run the notebook, save it and run nbdev_build_docs --fname nbs/index.ipynb from the terminal to update the README.md file. Let us know if you need any help.

@jmoralez Thank you. Will do it as you explained.

review-notebook-app · 2022-02-23T23:23:32Z

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

sugatoray · 2022-02-24T02:02:36Z

@jmoralez Done.

sugatoray · 2022-02-24T02:07:48Z

This PR aside, I am actually working on adding statsforecast to conda-forge channel. Although you have a conda installer, it does not help when some other package that depends on this library wants itself on conda-forge channel.

The conda-forge ecosystem is built to contain all necessary packages in the channel itself. Since, it is one of the largest channels on conda, it makes sense to add PyPI packages to conda-forge.

Adding statsforecast from PyPI conda-forge/staged-recipes#18136

Once the conda-forge PR is merged you will be able to install it with:

conda install -c conda-forge statsforecast

cc: @jmoralez @FedericoGarza

docs/index.html

jmoralez · 2022-02-24T03:21:27Z

nbs/index.ipynb

+    "[Getting started](https://github.com/Nixtla/statsforecast/tree/main/#-getting-started-) •\n",
+    "[Installation](https://github.com/Nixtla/statsforecast/tree/main/#-installation)\n",


Setting these as relative will allow them to work even on your branch so we can test them and (hopefully) in the docs as well.

Suggested change

"[Getting started](https://github.com/Nixtla/statsforecast/tree/main/#-getting-started-) •\n",

"[Installation](https://github.com/Nixtla/statsforecast/tree/main/#-installation)\n",

"[Getting started](#-getting-started-) •\n",

"[Installation](#-installation)\n",

But this is the source of the broken link on PyPI. Unless you somehow make them absolute links, I am not sure that problem goes away.

Ah I thought you meant they were broken in the github README. I agree with using the full links so they work on github, the webpage and PyPI. Can you please also change the section that says You can reproduce the results to use the github link as well?

jmoralez · 2022-02-24T03:23:39Z

README.md

@@ -121,7 +121,7 @@ pip install -e .

 ## 🧬 How to use

-```python
+```


Can you please re-render this and make sure that the python suffix stays there? Otherwise the syntax highlighting gets lost.

So, the nbdev rendering does not keep the python language specifier (L/S)? Do I have to manually ensure that the python L/S is still there (after the rendering)?

Did you save the executed notebook and ran the build_docs? The only reason I can think of that it would do this is if there's no metadata specifying the language in the notebook.

Ah! No, I did not execute the notebook. I just made the changes in the markdown and saved the file. And then ran the command you had mentioned.

Following #28 I think we may change the way we generate the README so if you want to fix the links as well we appreciate it but if this becomes too burdensome the changes in the manifest are already a huge help. Thank you!

I had a similar problem for one of my repositories. The hack that I implemented allowed me to keep relative links in readme file, but while publishing to PyPI, since it runs setup.py, the target set of links were updated with absolute path. This might give you an idea.

https://github.com/sugatoray/genespeak/blob/e6b49491cf5a872a03d55403ed34d2fa3469d0b8/setup.py#L7-L37

I am skipping updating the notebook/readme for now (reverted already). Please let me know if the rest is fine now.

- include/exclude files in source-distribution

jmoralez

Thank you very much!

sugatoray · 2022-02-27T01:51:02Z

@jmoralez If you just change the links from markdown format [![_description](_badge)](_target) to equivalent html, I think that should fix this. I am one of the contributors of seqlike. You might find the README.md of seqlike repo useful to fix this issue.

<a href="_TARGET_URL">
    <img src="_BADGE_URL" alt="_DESCRIPTION" />
</a>

seqlike/README.md: https://github.com/modernatx/seqlike/blob/75f3bf749df01aa0167e1e5d814f993220af23fd/README.md?plain=1#L18-L38

mergenthaler · 2022-02-28T23:44:17Z

@all-contributors please add @sugatoray for code

allcontributors · 2022-02-28T23:44:24Z

@mergenthaler

I've put up a pull request to add @sugatoray! 🎉

mergenthaler · 2022-02-28T23:46:18Z

@all-contributors please add @sugatoray for code

allcontributors · 2022-02-28T23:46:20Z

@mergenthaler

@sugatoray already contributed before to code

mergenthaler · 2022-02-28T23:47:05Z

@all-contributors please add @sugatoray for code

allcontributors · 2022-02-28T23:47:12Z

@mergenthaler

I've put up a pull request to add @sugatoray! 🎉

sugatoray changed the title ~~Add settings.ini to PyPI source~~ Adding settings.ini to PyPI source Feb 23, 2022

jmoralez requested changes Feb 24, 2022

View reviewed changes

sugatoray added 2 commits February 25, 2022 12:42

add .vscode to gitigore

a3e5a21

adding manifest.in

9451e45

- include/exclude files in source-distribution

sugatoray force-pushed the bugfix/packaging branch from e7a8d31 to 9451e45 Compare February 25, 2022 19:05

jmoralez approved these changes Feb 25, 2022

View reviewed changes

jmoralez added the ready to merge label Feb 25, 2022

sugatoray mentioned this pull request Feb 27, 2022

[docs] HTML in index notebook messes up online docs #28

Closed

AzulGarza merged commit ceb9ae0 into Nixtla:main Feb 28, 2022

allcontributors bot mentioned this pull request Feb 28, 2022

docs: add sugatoray as a contributor for code #38

Closed

allcontributors bot mentioned this pull request Feb 28, 2022

docs: add sugatoray as a contributor for code #39

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding `settings.ini` to PyPI source #24

Adding `settings.ini` to PyPI source #24

sugatoray commented Feb 23, 2022 •

edited

sugatoray commented Feb 23, 2022

jmoralez commented Feb 23, 2022

sugatoray commented Feb 23, 2022

review-notebook-app bot commented Feb 23, 2022

sugatoray commented Feb 24, 2022

sugatoray commented Feb 24, 2022 •

edited

jmoralez Feb 24, 2022

sugatoray Feb 24, 2022

jmoralez Feb 25, 2022

jmoralez Feb 24, 2022

sugatoray Feb 24, 2022 •

edited

jmoralez Feb 24, 2022

sugatoray Feb 24, 2022

jmoralez Feb 25, 2022

sugatoray Feb 25, 2022

jmoralez left a comment

sugatoray commented Feb 27, 2022 •

edited

mergenthaler commented Feb 28, 2022

allcontributors bot commented Feb 28, 2022

mergenthaler commented Feb 28, 2022

allcontributors bot commented Feb 28, 2022

mergenthaler commented Feb 28, 2022

allcontributors bot commented Feb 28, 2022

		"[Getting started](https://github.com/Nixtla/statsforecast/tree/main/#-getting-started-) •\n",
		"[Installation](https://github.com/Nixtla/statsforecast/tree/main/#-installation)\n",

Adding settings.ini to PyPI source #24

Adding settings.ini to PyPI source #24

Conversation

sugatoray commented Feb 23, 2022 • edited

sugatoray commented Feb 23, 2022

jmoralez commented Feb 23, 2022

sugatoray commented Feb 23, 2022

review-notebook-app bot commented Feb 23, 2022

sugatoray commented Feb 24, 2022

sugatoray commented Feb 24, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sugatoray Feb 24, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmoralez left a comment

Choose a reason for hiding this comment

sugatoray commented Feb 27, 2022 • edited

mergenthaler commented Feb 28, 2022

allcontributors bot commented Feb 28, 2022

mergenthaler commented Feb 28, 2022

allcontributors bot commented Feb 28, 2022

mergenthaler commented Feb 28, 2022

allcontributors bot commented Feb 28, 2022

Adding `settings.ini` to PyPI source #24

Adding `settings.ini` to PyPI source #24

sugatoray commented Feb 23, 2022 •

edited

sugatoray commented Feb 24, 2022 •

edited

sugatoray Feb 24, 2022 •

edited

sugatoray commented Feb 27, 2022 •

edited