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

Refactor setup.py. Add setup.cfg. modify issue & PR templates. #1096

Merged
merged 18 commits into from
Jan 12, 2021

Conversation

Silverarmor
Copy link
Member

Resolves #1095

@Silverarmor Silverarmor linked an issue Jan 9, 2021 that may be closed by this pull request
setup.cfg Show resolved Hide resolved
}
)
if __name__ == "__main__":
setup()
Copy link
Member Author

Choose a reason for hiding this comment

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

@Silverarmor Silverarmor requested a review from a user January 10, 2021 00:31
@Silverarmor Silverarmor marked this pull request as ready for review January 10, 2021 00:31
@Silverarmor Silverarmor requested a review from aklajnert January 10, 2021 00:32
@Silverarmor Silverarmor added the Enhancement Enhancing spotDL label Jan 10, 2021
@Silverarmor Silverarmor mentioned this pull request Jan 10, 2021
Copy link
Member Author

@Silverarmor Silverarmor left a comment

Choose a reason for hiding this comment

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

Have tested to https://test.pypi.org/project/spotdltester
All is working.
All data migrated from setup.py to setup.cfg

@Silverarmor
Copy link
Member Author

Attention future contributor making changes to setup.py:

For whoever reads this in the future, updating setup.py, setup.cfg, setup.toml or similar:
The documentation on using the new setup.cfg is really bad. Hopefully when you are reading this, the documentation has improved.
The setuptools readthedocs is probably your best bet Or you can try the quickstart guide on the setuptools readthedocs

The Python Packaging tutorial documentation is outdated at time of writing, still saying to use a setup.py, but setup.py contains the same data, in a different format.
Python docs website, about metadata. Again, outdated, but similar data types. Often a comma list rather than semicolon separated list etc.

Python documentation for the config file. Literally useless. It's the page we need, but it contains 5% of the information we actually need.

Literally the most help was this stackoverflow answer

Eventually,

We will need to migrate from a setup.py to a setup.toml, as per PEP 517 / 518

But quote the stack overflow answer,

That being said, the landscape of Python packaging software is very much in motion at the moment (2019/2020) and it is not clear which will be the preferred method of defining a Python package in the future. For example, there is PEP 621 which suggests to put package metadata into pyproject.toml.


Remember that this post was made in January 2021. Packaging python projects is in the middle of a big change. It is likely some of this information is outdated. Fingers crossed, documentation is better when you are contributing to the spotDL project.

@aklajnert
Copy link
Contributor

I'd suggest merging #1039 first, then update this branch and see if the tests still pass in a clean environment for each supported interpreter.

@Silverarmor
Copy link
Member Author

Assuming you mean #1093...

@aklajnert
Copy link
Contributor

Assuming you mean #1093...

Oh, right - typo.

Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

It's clearer if we list only top-level, directly imported dependencies. Apart from that all good.

setup.cfg Outdated
download_url = https://pypi.org/project/spotdl/

author = spotDL Team
author_email = unrealengin71+PyPi@gmail.com
Copy link

Choose a reason for hiding this comment

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

Change the email to the googlegroups id

setup.cfg Outdated
pytube
rich
rapidfuzz
requests
Copy link

Choose a reason for hiding this comment

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

We don't explicitly use requests, just list the top level requirements

setup.cfg Show resolved Hide resolved
@Silverarmor
Copy link
Member Author

@aklajnert can you lend some insight into these checks that are failing?

@aklajnert
Copy link
Contributor

@aklajnert can you lend some insight into these checks that are failing?

  __________________ ERROR collecting tests/test_entry_point.py __________________
  ImportError while importing test module '/home/runner/work/spotify-downloader/spotify-downloader/tests/test_entry_point.py'.
  Hint: make sure your test modules/packages have valid Python names.
  Traceback:
  /opt/hostedtoolcache/Python/3.7.9/x64/lib/python3.7/importlib/__init__.py:127: in import_module
      return _bootstrap._gcd_import(name[level:], package, level)
  tests/test_entry_point.py:6: in <module>
      from spotdl.__main__ import console_entry_point, help_notice
  spotdl/__main__.py:10: in <module>
      from spotdl.download.downloader import DownloadManager
  spotdl/download/downloader.py:22: in <module>
      from spotdl.download.progressHandlers import DisplayManager, DownloadTracker
  spotdl/download/progressHandlers.py:7: in <module>
      from tqdm import tqdm
  E   ModuleNotFoundError: No module named 'tqdm'

You don't have tqdm in the requirements.

@Silverarmor
Copy link
Member Author

Must've slipped my mind. will add it now.

@Silverarmor Silverarmor requested a review from a user January 11, 2021 11:06
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

👍

setup.cfg Show resolved Hide resolved
@Silverarmor
Copy link
Member Author

I'd suggest merging #1039 first, then update this branch and see if the tests still pass in a clean environment for each supported interpreter.

Done. Could you have a quick review?

@Silverarmor Silverarmor changed the title Refactor setup.py - Transition to setup.cfg rather than setup.py Refactor setup.py. Add setup.cfg. modify issue & PR templates. Jan 12, 2021
@Silverarmor Silverarmor merged commit a40dc14 into spotDL:dev Jan 12, 2021
@ghost
Copy link

ghost commented Jan 12, 2021

Also, On squash merges, add a

Author: @author1-github-handle
Co-Authors: @author2-github-handle, @author3-github-handle...

near the top

@Silverarmor Silverarmor deleted the setup-fix branch January 12, 2021 09:06
Silverarmor added a commit that referenced this pull request Jan 14, 2021
Finalizing Development branch
--
* Improved `saveFile` handling

* Fixed tests, recorded new cassettes

* Setup CI on GitHub actions

* Disable VCR only for Python 3.8 to speed up tests

* Clean `mypy` and add it to the CI

* Fixed syntax for tox.ini

* Refactor setup.py. Add setup.cfg. modify issue & PR templates. (#1096)

* Transition to setup.cfg rather than setup.py

* Update setup.cfg

* add packages

* Add Readme to PyPi

* Spacing

* Delete oldsetup.py

* Drop Version Support for Py3.6

* Update as for #1098

* Fix Critical Error in setup.py

Had comma separated list rather than semi colon. Prevented setup.py from functioning

* Add 3.9 classifier

* Update setup.cfg

* Drop py3.6 support

* Re-add Py3.6 support

* Change Email

* Remove `requests`

* add tqdm to requirements

* Fix Line Lengths in bug report template

it was annoying me lol

* Update and rename pull_request_template.md to PULL_REQUEST_TEMPLATE.md

* Fixing typo (#1108)

@last72 helping to fix typos in CONTRIBUTING.md

* Fixing typo

ometimes, -> Sometimes,

* Fixing typo

intensions -> intentions

* Core values: How we decided what gets included (#1105)

@MikhailZex - Core values: How we decided what gets included


* Values: What get added, and what gets removed

* Updates based on @Silverarmor's Review

- Used title case
- Canged '~(80%)' to '~80%+)
- Removed unnecessary line breaks
- Removed extra COREVALUES.md

* Prepare Update to 3.3.0

Updated setup.cfg version to 3.3.0

* Fix setup.cfg & setup.py dev (#1116)

Authored by @phcreery @Silverarmor 
@phcreery helped solve the big problem!

* Attempt to unfck it

* Try change entry point to all?

* Try add a main() function

* remove colon bit

* Revert to 172e973
Signed-off-by: Silverarmor <23619946+Silverarmor@users.noreply.github.com>

* idk whats happening

* try fix packages? mayb helps?

* back to __main__

* Removed where to look

* script as spotdl

* didnt work, trying spotdl:spotdl

* trying __main__ maybe?

* I am literally desperate and trying different files now.

hopefully? maybe?

* hopefully fixed

* Revert "hopefully fixed"

This reverts commit 1f07c0b.

* Fix entry point.

Big thanks @phcreery who figured this one out!

Co-Authored-By: Peyton Creery <44987569+phcreery@users.noreply.github.com>
Co-authored-by: Peyton Creery <44987569+phcreery@users.noreply.github.com>

* Transfer mypy.ini to setup.cfg

Co-authored-by: Andrzej Klajnert <github@aklajnert.pl>
Co-authored-by: Woongyeol Choi <cwy5847@gmail.com>
Co-authored-by: Michael George <MikhailZex@gmail.com>
Co-authored-by: Peyton Creery <44987569+phcreery@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Enhancing spotDL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor setup.py
2 participants