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

Add pyproject.toml and packaging config #10

Merged
merged 6 commits into from
Oct 21, 2024

Conversation

luizirber
Copy link
Collaborator

@luizirber luizirber commented Oct 14, 2024

Add pyproject.toml with setuptools build backend, and use uv to verify package works.

Likely review this after #9 is merged, and how the requirements.txt file is generated

@jeff-cohere
Copy link
Collaborator

@ialarmedalien , Luiz Irber is one of our BER Data Integration Shenanigans colleagues, and another contractor type hired by Kjiersten to Do Stuff. He believes in reducing friction, and has a bit more familiarity with the miasma of Python-related package management tools. Can you comment on how uv, etc, fits in with other KBase tools of interest?

@jeff-cohere
Copy link
Collaborator

The tests are failing for this PR because the DTS_KBASE_DEV_TOKEN secret isn't available to PRs from a fork.

@luizirber luizirber marked this pull request as ready for review October 17, 2024 17:39
@ialarmedalien
Copy link
Collaborator

General point: let's just standardise to using py3.12 instead of having 3.11 in some places and 3.12 in others.

Comment on lines +14 to +17
dependencies = [
"frictionless>=5.17.0,<6",
"requests>=2.32.3,<3",
]
Copy link
Collaborator

Choose a reason for hiding this comment

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

have you tried running dtspy from a clean py3.12 install? The requirements file is quite long - are these the only two deps? I hope that the testing and doc generation aren't responsible for all the rest!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These two are the top-level dependencies, most of the ones listed in the requirements file come from them indirectly.

To see the equivalent requirements.txt based on the uv lock file, you can run
uv pip compile pyproject.toml
(and maybe add --no-annotate to make it easier to diff with current requirements.txt)

But yes, there are many that are testing/doc deps...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And I'm already using as a dep in a project with

[project]
dependencies = [
  "dtspy @ https://github.com/luizirber/dtspy/archive/724f2884e14fee4cc9f9019e3bdf63a46aecf487.tar.gz"
]

which points to this PR, and it is all working

@ialarmedalien
Copy link
Collaborator

We need either an additional PR or amendments to this PR that do the following:

  • separate out the deps into runtime, testing, and documentation generation -- in particular, there seem to be a ton of documentation things that people are not going to need if they run the code locally
  • additional information in the README about how to install and run the project locally -- currently there's no documentation at all and only people who are already python-savvy or who want to pick through the GitHub actions will work out how to do it.

@luizirber we appreciate your contribution and don't want to dump a load of extra work on you so can you and @jeff-cohere divide and conquer those tasks between yourselves?

@luizirber
Copy link
Collaborator Author

We need either an additional PR or amendments to this PR that do the following:

* separate out the deps into runtime, testing, and documentation generation -- in particular, there seem to be a ton of documentation things that people are not going to need if they run the code locally

This is somewhat done already, dev deps are listed here:
https://github.com/kbase/dtspy/pull/10/files#diff-50c86b7ed8ac2cf95bd48334961bf0530cdc77b5a56f852c5c61b89d735fd711R19-R26

I can add another level to be doc vs testing deps

* additional information in the README about how to install and run the project locally -- currently there's no documentation at all and only people who are already python-savvy or who want to pick through the GitHub actions will work out how to do it.

Agreed, will add to this PR!

@luizirber we appreciate your contribution and don't want to dump a load of extra work on you so can you and @jeff-cohere divide and conquer those tasks between yourselves?

👍

@jeff-cohere
Copy link
Collaborator

Thanks, both. I find the Python and Javascript communities to be similarly ultra-aggressive in terms of dependencies, and I haven't really formed my own philosophy about how to manage the resuting pain that these communities feel is a cost of doing business.

@luizirber , thanks for taking point on this particular topic for the moment. Happy to kick ideas around.

@jeff-cohere
Copy link
Collaborator

additional information in the README about how to install and run the project locally -- currently there's no documentation at all and only people who are already python-savvy or who want to pick through the GitHub actions will work out how to do it.

This is on my list for the very next PR!

@ialarmedalien
Copy link
Collaborator

Separate deps segments for mkdocs and testing would be great, thanks. Something super simple would be fine for the readme -- @jeff-cohere can always flesh it out in his upcoming PR.

@jeff-cohere jeff-cohere merged commit b96ce5a into kbase:main Oct 21, 2024
2 of 4 checks passed
@luizirber luizirber deleted the lirber/packaging branch October 23, 2024 21:48
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.

3 participants