-
Notifications
You must be signed in to change notification settings - Fork 0
refactor: ♻️ match Sprout #40
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
Conversation
| To add uv: | ||
|
|
||
| 1. Delete the `pyproject.toml` file. | ||
| 2. In the terminal run `uv init`. | ||
| 3. Look at the Git pane and take what was removed and move it over into | ||
| 1. Delete the `pyproject.toml` file. | ||
| 2. In the terminal run `uv init`. | ||
| 3. Look at the Git pane and take what was removed and move it over into | ||
| the new `pyproject.toml` file. You can mimic what was done in | ||
| `example-seed-beetle` repo. | ||
|
|
||
| Then, in the terminal, run: | ||
|
|
||
| ``` bash | ||
| uv add polars pyjanitor | ||
| uv add "seedcase-sprout @ git+<https://github.com/seedcase-project/seedcase-sprout>" | ||
| uv add seedcase-sprout@git+https://github.com/seedcase-project/seedcase-sprout | ||
| uv add --dev ruff commitizen pre-commit typos | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As the pyproject.toml file is already set up, could we not just run uv sync?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. Though we'll very soon need to set the pypi version of Sprout, not the GitHub version. That's something you can do here.
main.py
Outdated
| # Write properties from properties script to `datapackage.json`. | ||
| # sp.write_properties(properties=properties) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could also add the resources bit commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a lot more that can be added here, based on what is in Sprout's guide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah okay! I didn't know whether the intention was to include as much of the guide or not
| To add uv: | ||
|
|
||
| 1. Delete the `pyproject.toml` file. | ||
| 2. In the terminal run `uv init`. | ||
| 3. Look at the Git pane and take what was removed and move it over into | ||
| 1. Delete the `pyproject.toml` file. | ||
| 2. In the terminal run `uv init`. | ||
| 3. Look at the Git pane and take what was removed and move it over into | ||
| the new `pyproject.toml` file. You can mimic what was done in | ||
| `example-seed-beetle` repo. | ||
|
|
||
| Then, in the terminal, run: | ||
|
|
||
| ``` bash | ||
| uv add polars pyjanitor | ||
| uv add "seedcase-sprout @ git+<https://github.com/seedcase-project/seedcase-sprout>" | ||
| uv add seedcase-sprout@git+https://github.com/seedcase-project/seedcase-sprout | ||
| uv add --dev ruff commitizen pre-commit typos | ||
| ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True. Though we'll very soon need to set the pypi version of Sprout, not the GitHub version. That's something you can do here.
main.py
Outdated
| # Write properties from properties script to `datapackage.json`. | ||
| # sp.write_properties(properties=properties) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a lot more that can be added here, based on what is in Sprout's guide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assumed this was a hangover and deleted it
| # from scripts.resource_properties import resource_properties | ||
|
|
||
|
|
||
| def main(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be the full script. You can't just uncomment and run everything at once because required properties need to be filled in, scripts/resource_properties.py needs to be created, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing! Just one comment, which I will commit and then will approve and merge 🎉
Description
This PR:
uv cache cleanto make it pull the newest one)main.pyIs there anything else to do to make the template match Sprout? Sorry if I completely misunderstood it, but it feels like it's ready to use the latest version of Sprout.
I could add
mypy?Closes #29, closes #33
This PR needs a quick review.
Checklist
just run-all