Skip to content

Switch from pip to poetry - #1358

Closed
Nimrais wants to merge 19 commits into
catalyst-team:masterfrom
Nimrais:switch-from-pip-to-poetry
Closed

Switch from pip to poetry#1358
Nimrais wants to merge 19 commits into
catalyst-team:masterfrom
Nimrais:switch-from-pip-to-poetry

Conversation

@Nimrais

@Nimrais Nimrais commented Nov 23, 2021

Copy link
Copy Markdown
Contributor

Pull Request FAQ

Description

Related Issue

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)

PR review

Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

Checklist

  • Have you updated tests for the new functionality?
  • Have you added your new classes/functions to the docs?
  • Have you updated the CHANGELOG?
  • Have you run colab minimal CI/CD with latest and minimal requirements?
  • Have you checked XLA integration with single and multiple processes?

@CLAassistant

CLAassistant commented Nov 23, 2021

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@bagxi

bagxi commented Nov 23, 2021

Copy link
Copy Markdown
Member

Hi @Nimrais ,
Could you please fill me in if Poetry supports entry_points and scripts? I guess it will be hard to switch to poetry without those features:(

@Nimrais

Nimrais commented Nov 23, 2021

Copy link
Copy Markdown
Contributor Author

Hi @Nimrais ,
Could you please fill me in if Poetry supports entry_points and scripts? I guess it will be hard to switch to poetry without those features:(

Yes, poetry has scripts.
And I think no problems with entry points as well.

@bagxi

bagxi commented Nov 23, 2021

Copy link
Copy Markdown
Member

I guess by scripts they mean Python scripts. But in Catalyst, we also have a bunch of bash scripts, and last time I checked (in September) there was no easy way to add bash scripts to the package.

@Nimrais

Nimrais commented Nov 23, 2021

Copy link
Copy Markdown
Contributor Author

I guess by scripts they mean Python scripts. But in Catalyst, we also have a bunch of bash scripts, and last time I checked (in September) there was no easy way to add bash scripts to the package.

I guess by scripts they mean Python scripts. But in Catalyst, we also have a bunch of bash scripts, and last time I checked (in September) there was no easy way to add bash scripts to the package.

I think no. Yeah, adding a bash script with poetry is a bit complicated.
But it's possible. I will test it and will return it to you.

Also, several poetry plugins can do this work for us.
For example, https://pypi.org/project/poethepoet/.

You can define a task with it (it can be almost everything):

[tool.poe.tasks]
hellow-task = {"shell" = "echo 'Hellow world'"}

And execute it:

poe hellow-task

@bagxi

bagxi commented Nov 23, 2021

Copy link
Copy Markdown
Member

Could you please test it?
I hope there are some ways to do it without forcing users to install Poetry. Just pip install catalyst + catalyst-dl run ...

@Nimrais

Nimrais commented Nov 24, 2021

Copy link
Copy Markdown
Contributor Author

Could you please test it?
I hope there are some ways to do it without forcing users to install Poetry. Just pip install catalyst + catalyst-dl run ...

Yes, adding bash script into a poetry project is possible but it can be accessed only with poetry run <command> from the best of my understanding.

I think this use way (<command>) is possible only if a script is written in python (e.g. we can wrap all our scripts with python: we have only 4).
So if you have smt like that:

├── cli_scripts
│   ├── __init__.py
│   ├── location.py

And a function hellow inside location.py

pyproject.toml:

[tool.poetry.scripts]
hellow = "cli_scripts.location:hellow"

After installation of the project with pip you will obtain hellow in your command line:

(cli-scripts-9hiUP3k7-py3.8) mykola@catalyst:~/cli_scripts$ hellow
hi

@bagxi

bagxi commented Nov 24, 2021

Copy link
Copy Markdown
Member

Great, thank you)

And I think it is time to summon ༼ つ ◕_◕ ༽つ @Scitator ༼ つ ◕_◕ ༽つ

@Scitator

Copy link
Copy Markdown
Member

Our current 4 bash scripts are not well documented and mostly used for contribs purposes, so I don't think it would be too pity to delist them for a while... + you still could copy-paste them if they are truly needed - we have a license in them ;)

@mergify

mergify Bot commented Nov 29, 2021

Copy link
Copy Markdown

This pull request is now in conflicts. @Nimrais, could you fix it? 🙏

@stale

stale Bot commented Jan 28, 2022

Copy link
Copy Markdown

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale Bot added the wontfix This will not be worked on label Jan 28, 2022
@stale stale Bot closed this Feb 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

wontfix This will not be worked on

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants