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

WIP: ENH: Add script to migrate setup.py to pyproject.toml #161

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

thewtex
Copy link
Member

@thewtex thewtex commented Jun 11, 2024

Contains a few assumptions, but will generate a good starting point for most
cases.

Contains a few assumptions, but will generate a good starting point for most
cases.
@jhlegarreta
Copy link
Member

jhlegarreta commented Jun 22, 2024

Thanks for doing this Matt 💯.

Gave it a try in InsightSoftwareConsortium/ITKTubeTK#162:

  • The long_description is not parsed or is parsed in lieu of description and in the latter case multi-line descriptions are not handled correctly. The former should correspond now to the readme field I guess, so besides fixing the bug maybe printing a note that it is replaced by the value of readme could be helpful.
  • The raw strings (i.e. starting with an r) in setup.py are not parsed correctly and make the script exit without success.
  • The dependencies are not parsed correctly (in this case, maybe influenced by the raw strings and maybe due to each dependency being on a separate line and that introducing whitespaces).
  • Keywords are not being parsed.
  • Do we want to add Repository and Issues to the cookiecutter template and be also parsed here?
  • Does authors introduce unnecessary whitespaces before name and before the closing bracket after email?
  • Should all keywords be made lowercase automatically?

@jhlegarreta
Copy link
Member

We will probably need additional steps to add the documentation dependencies, e.g.
https://github.com/InsightSoftwareConsortium/ITKTubeTK/blob/42104e631e8ef88d36144332a8bcae4070ab0fa7/docs/requirements.txt#L3

Also, we may need to warn users to add any additional dev or testing dependencies to [dev] or [test] etc. (e.g. "pre-commi"t, etc.).

template = template.replace('{{ cookiecutter.download_url }}', project_url)

setup_dirname = os.path.dirname(setup_python_path)
if os.path.exists(os.path.join(setup_dirname, 'README.md')):
Copy link
Member

Choose a reason for hiding this comment

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

Eventually, all README files should be translated to either markdown or restructuredtext 🙃.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed. We could do this as a follow-up step.

@thewtex
Copy link
Member Author

thewtex commented Jun 23, 2024

@jhlegarreta thank you for the review and testing!

I will knock some of these out next time I exercise the script. Feel free to push to this branch, though, if you have fixes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants