-
-
Notifications
You must be signed in to change notification settings - Fork 374
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] Fixes: #3928 Added First Class Python Support #3986
[WIP] Fixes: #3928 Added First Class Python Support #3986
Conversation
@lihaoyi Good Wishes Sir! Actually, i have done some workaround after carefully checking other examples and everything is exact same but the way i used for testing is far different from the other examples. I don't know this is right or not (as per mill structure), Require your Guidance Please Review and Guide me... |
@lihaoyi, If Possible could you please review this PR coz further work requires your attention so please once provide the updates required so that i could bring this to mark... |
|
||
trait PythonTests extends PythonModule { | ||
|
||
def pythonUnitTests: T[Unit] = Task { |
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 a Task.Command
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.
Let's also call it def test
for consistency with the other languages
stdout = os.Inherit | ||
) | ||
|
||
PathRef(pexFile) | ||
} | ||
|
||
def pythonRepl(args: mill.define.Args) = Task.Command { |
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 probably be exclusive = true
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.
We can also just call it repl
for consistency with Scala
@@ -0,0 +1,39 @@ | |||
//// SNIPPET:BUILD |
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.
We don't need these snippet comments here since we're not sharing example code with the other languages
object foo extends PythonModule { | ||
object bar extends PythonModule { | ||
def pythonDeps = Seq("pandas==2.2.3", "numpy==2.1.3") | ||
def pythonDeps = Seq("pandas==2.2.3", "numpy==2.1.3") |
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.
Let's get rid of pandas and numpy in these examples; they're really heavyweight packages and take a long time to download and install. We use HTML templating in our Java/Scala/Kotlin examples, so let's do so here as well. Jinja2 seems like a reasonable choice
|
||
object test extends PythonTests { | ||
def moduleDeps = Seq(foo) | ||
def mainFileName: T[String] = Task { "test.py" } |
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.
What does the test module require a mainFileName
? Shouldn't the test framework discover the files automatically?
def pythonDeps = Seq("pandas==2.2.3", "numpy==2.1.3") | ||
|
||
object test extends PythonTests { | ||
def moduleDeps = Seq(foo) |
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.
The test module should not need an explicit dependency on the enclosing parent module. It should be automatic like it is in Java/Scala/Kotlin
def pythonDeps = Seq("pandas==2.2.3", "numpy==2.1.3") | ||
def pythonDeps = Seq("pandas==2.2.3", "numpy==2.1.3") | ||
|
||
object test extends PythonTests { |
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 have an example of a third-party dependency being used in the test suite as well
@lihaoyi tbh all reviews are the expected ones coz i myself was not satisfied with this way of testing and want to acheive something like in other examples. But the problem is with docs i didn't found anything related to this to get a learning spike... If possible could you please provide me some docs link or guide on this please... |
closing in favour of #4000 |
Pull Request
Added First Class Python Support
Fixes: #3928
Description
Adds Example for Python
1-simple
,2-custom-build-logic
and3-multi-module
SkeletonDocumentation Not Updated Yet!
Related Issues
Checklist
1-Simple
2-custom-build-logic
3-multi-module
Status
Require Reviews and Further fine-tuning