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

Some improvements to pythonlib #3992

Merged
merged 13 commits into from
Nov 20, 2024
Merged

Conversation

jodersky
Copy link
Member

@jodersky jodersky commented Nov 19, 2024

This is in view of #3928, specifically some essential things required for the first task, example/pythonlib/basic/.

You can look at commits individually if you like. The gist of this PR is to improve the scaffolding we currently have to unblock the previously mentioned tasks. It can be summarized by the following points (roughly each corresponding to one commit):

  • make run task the default, following the same convention used in ScalaModule and JavaModule
  • add a command to run an interactive REPL. This is named console, again following the conventions of scalalib
  • rework the way source files are handled:
    • support multiple source directories, again following similar conventions of scalalib
    • instead of aggreting python scripts in one syntetic directory, keep them where they are and instead manipulate PYTHONPATH or pass in parameters to various directories where appropriate
  • define a module for unit tests, similar to the TestModule of scalalib

- support multiple source directories per mill module
- python modules are relative to source directories
- avoid gathering all scripts in a single directory (that's a lot of copying!) and instead use PYTHONPATH and friends
@himanshumahajan138
Copy link
Contributor

@jodersky TBH this is what i was looking for, Great !!!

Also i want to add something like the way i used Repl i think that is also good See #3986

And the way of testing is now professional and matching with other examples (i didn't liked my way of testing)

So i think i should wait till this PR gets Merged and after wards i will work further on #3986 till then let me copy your changes and work around with the same pattern

Thanks...

@jodersky
Copy link
Member Author

jodersky commented Nov 20, 2024

@himanshumahajan138, that sounds good. I'll finish up test support, then we can hopefully merge, and you take it from there to finish the example/pythonlib/basic/ support of #3928.

I can continue on example/pythonlib/dependencies/ of the same issue then, if that's ok with you

@himanshumahajan138
Copy link
Contributor

@jodersky ya that's great we will work together to bring the professional python first class support to mill

Team Work is the Dream Work !!!

@himanshumahajan138
Copy link
Contributor

himanshumahajan138 commented Nov 20, 2024

./mill mill.scalalib.scalafmt.ScalafmtModule/
./mill mill.javalib.palantirformat.PalantirFormatModule/
./mill mill.kotlinlib.ktlint.KtlintModule/

@jodersky these commands will fix the lint error automatically from the root

@jodersky
Copy link
Member Author

Alright, I think this is ready for review now.

@jodersky jodersky marked this pull request as ready for review November 20, 2024 12:48
@lihaoyi
Copy link
Member

lihaoyi commented Nov 20, 2024

Looks pretty good overall. Left some comments. @jodersky you mentioned you looked at how Bazel handles Python, if in that research you found anything relevant to this PR can summarize whatever you learned that's relevant in the PR description

@himanshumahajan138
Copy link
Contributor

@jodersky i think we should work on linting before deps coz deps don't require much logical and backend code but linting is far harder and more complicated and in that case, experience is more required 😉

Rest is You's Choice...

@jodersky
Copy link
Member Author

jodersky commented Nov 20, 2024

@jodersky i think we should work on linting before deps coz deps don't require much logical and backend code but linting is far harder and more complicated and in that case, experience is more required 😉

I'm not sure. I think dependencies are a very important feature, and they aren't all that easy when you consider that we'll need more than just building a pip command line.

For example, we'll need to deal with things like installing wheels. Also, there are a couple of "quality-of-life" features that I'd like to include:

  • ability to specifcy dependencies via an existing requirements.txt file (this is inspired by pants' python support)
  • on dependency conflicts, mention which modules requested the conflicting version (right now, since we're only building a pip command line, it's really hard to find out what module is the culprit)

@himanshumahajan138
Copy link
Contributor

@jodersky i think we should work on linting before deps coz deps don't require much logical and backend code but linting is far harder and more complicated and in that case, experience is more required 😉

I'm not sure. I think dependencies are a very important feature, and they aren't all that easy when you consider that we'll need more than just building a pip command line.

For example, we'll need to deal with things like installing wheels. Also, there are a couple of "quality-of-life" features that I'd like to include:

  • ability to specifcy dependencies via an existing requirements.txt file (this is inspired by pants' python support)
  • on dependency conflicts, mention which modules requested the conflicting version (right now, since we're only building a pip command line, it's really hard to find out what module is the culprit)

Ok that's great afterall this all wil be available for mill 😊

@jodersky
Copy link
Member Author

@lihaoyi, I think I addressed all your comments. Would you like to go over it once more? Regarding the python example, it will all be reworked by @himanshumahajan138.

@lihaoyi
Copy link
Member

lihaoyi commented Nov 20, 2024

@jodersky looks great, I'll merge it and we can continue iterating on it after

@lihaoyi lihaoyi merged commit 4729afc into com-lihaoyi:main Nov 20, 2024
27 checks passed
@lefou lefou added this to the 0.12.3 milestone Nov 21, 2024
lihaoyi pushed a commit that referenced this pull request Nov 26, 2024
# Pull Request 
Added First Class Python Support [Basic Example]
Fixes: #3928

## Description
Rework for Python `1-simple`, `2-custom-build-logic` and
`3-multi-module` Examples
follow up for PR #3992 

## Related Issues
- Link to related issue #3928.

## Checklist
- [x] New Example for Python `1-Simple`
- [x] New Example for Python `2-custom-build-logic`
- [x] New Example for Python `3-multi-module`
- [x] Updated Documentation

## Status
Completed Addition & Require Review!!!
@jodersky jodersky added the pythonlib Issues related to Mill's python support label Nov 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pythonlib Issues related to Mill's python support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants