https://pantsbuild.org/ logo
Join Slack
Powered by
Is there an existing mechanism in `python_distribu...
# plugins
w
Is there an existing mechanism in 
python_distribution
to add imports to the setup.py? in 
setup_py.py
we have the following:
Copy code
SETUP_BOILERPLATE = """
# DO NOT EDIT THIS FILE -- AUTOGENERATED BY PANTS
# Target: {target_address_spec}

from setuptools import setup

setup(**{setup_kwargs_str})
"""
but if I need another import in there, can that be added via the BUILD file? Or do I just need to use a custom setup.py/auto-generate my own
Essentially, I was hoping to use the SetupKwargsRequest plugin API to allow me to generate this under a new target (which is almost identical to the 
python_distribution
target)
Copy code
from setuptools import setup

from mypyc.build import mypycify

setup(
    name="hellofib",
    packages=["hellofib"],
    ext_modules=mypycify(
        [
            "hellofib/__init__.py",
            "hellofib/main.py",
        ]
    ),
)
When I have that setup.py in my root folder, it can't find mypy, which makes sense in the packaging workflow. As we don't have a compile workflow, leaving this under the python_distribution seems reasonable. So, my plugin was going to add mypy to this target, and then ideally just change the SetupKwargs to stay as DRY as possible. But, it's looking like I'll need to be able to add my own 
generate_setup_py
method. Does that exist natively? Or would that extension need to be added?
At the moment, I'm trying to create my own
Copy code
@rule
async def generate_setup_py(request: GenerateSetupPyRequest) -> GeneratedSetupPy:
as that seems like the correct appraoch
Unfortunately, this falls over with a bunch of graph rule errors
Related to this specifically - I need to be able to overwrite this method, but when I try to setup union rules and monkey patch and add unions to the lower-level classes, it's a no go https://github.com/sureshjoshi/pants-plugins/blob/55602edc19ac4113f1dd1582e9e513ed662a8c77/pants-plugins/experimental/mypyc/rules.py#L83-L100
b
I'm kind of out of my element here, but sounds like you might need to edit the Pants rules to allow for additional info being passed in to the request, and then used to make the 
setup.py
file?
I wouldn't expect the Pants engine to be very tolerant of overwriting or monkeypatching 🙂
w
Haha, the monkey patch was just to see if anything was working the way I thought it was 🙂
b
Seems trivial to add to Pants the ability to specify additional kwargs to include in the generated 
setup
call
If you want quick hacking, you can use 
./pants_from_sources
😉
w
I've been editing my cached pants 😅 So, the kwargs are currently working (sorta) in the mypyc_setup_kwargs function - but I need to make sure mypyc is imported, and then make sure mypycify is available as a function in the setup.py - that's where the new setup.py file comes from
b
Ah so even more metadata is required 😉
Looks like you have your work cut out for you
I've been editing my cached pants
That's dangerous, you won't have the ability to back up, and changes could get overblown accidentally 😉
w
Yeah, it's just one file that I'm only adding some 
@unions
to and logging - so nothing I'll forget about or anything worthwhile.
✅ 1
b
Actually, I forgot about Unions!
OK so this gets outside of my expertise, but there might be solution where you change Pants to Union some of the inputs to generation. Unclear to me
w
So, if I just try to re-write generate_setup_py, I get:
Copy code
ValueError: Encountered 1 rule graph error:
  Too many sources of dependency Get(GeneratedSetupPy, GenerateSetupPyRequest) for @rule(pants.backend.python.goals.setup_py:481:generate_chroot(DistBuildChrootRequest, SetupPyGeneration) -> DistBuildChroot, gets=[Get(DistBuildSources, DistBuildChrootRequest), Get(GeneratedSetupPy, GenerateSetupPyRequest), Get(PyDigest, PyMergeDigests), Get(StrippedSourceFileNames, SourcesPaths)]): [
        "@rule(experimental.mypyc.rules:56:generate_setup_py(GenerateSetupPyRequest) -> GeneratedSetupPy, gets=[Get(FinalizedSetupKwargs, GenerateSetupPyRequest), Get(PyDigest, CreateDigest)]) (for GenerateSetupPyRequest)",
        "@rule(pants.backend.python.goals.setup_py:518:generate_setup_py(GenerateSetupPyRequest) -> GeneratedSetupPy, gets=[Get(FinalizedSetupKwargs, GenerateSetupPyRequest), Get(PyDigest, CreateDigest)]) (for GenerateSetupPyRequest)",
    ]
That's a perfectly clear error to me.
So, then I modify my inputs accordingly
Copy code
class MyPycGenerateSetupPyRequest(GenerateSetupPyRequest):
    pass


@rule(level=LogLevel.DEBUG)
async def generate_setup_py(request: MyPycGenerateSetupPyRequest) -> GeneratedSetupPy:
    # Generate the setup script.
    <http://logger.info|logger.info>("Inside the custom generate_setup_py function")
    finalized_setup_kwargs = await Get(
        FinalizedSetupKwargs, MyPycGenerateSetupPyRequest, request
b
You re-defining a function with the same inputs/outputs confuses the engine. Which implementation should it choose?
This will assuredly require some change to Pants itself
w
After I modify my inputs, it doesn't know to even look at mine (naturally), so I add a UnionRule to let it know. It complains that GenerateSetupPyRequest or GeneratedSetupPy aren't unions, so I hack some on (cough, in my cached pants), and then add UnionRules accordingly.
And then 💥
Copy code
ValueError: Encountered 6 rule graph errors:
  No installed rules return the type GenerateSetupPyRequest, and it was not provided by potential callers of @rule(pants.backend.python.goals.setup_py:518:generate_setup_py(GenerateSetupPyRequest) -> GeneratedSetupPy, gets=[Get(PyDigest, CreateDigest)]).
    If that type should be computed by a rule, ensure that that rule is installed.
    If it should be provided by a caller, ensure that it is included in any relevant Query or Get.
  No installed rules return the type GenerateSetupPyRequest, and it was not provided by potential callers of @rule(pants.backend.python.goals.setup_py:534:generate_setup_py_kwargs(GenerateSetupPyRequest) -> FinalizedSetupKwargs, gets=[Get(ExportedTargetRequirements, DependencyOwner), Get(SetupKwargs, ExportedTarget), Get(ResolvedPythonDistributionEntryPoints, ResolvePythonDistributionEntryPointsRequest)]).
    If that type should be computed by a rule, ensure that that rule is installed.
    If it should be provided by a caller, ensure that it is included in any relevant Query or Get.
  No source of dependency Get(ExportedTargetRequirements, DependencyOwner) for @rule(pants.backend.python.goals.setup_py:534:generate_setup_py_kwargs(GenerateSetupPyRequest) -> FinalizedSetupKwargs, gets=[Get(ExportedTargetRequirements, DependencyOwner), Get(SetupKwargs, ExportedTarget), Get(ResolvedPythonDistributionEntryPoints, ResolvePythonDistributionEntryPointsRequest)]). All potential sources were eliminated: []
  No source of dependency Get(PyDigest, CreateDigest) for @rule(pants.backend.python.goals.setup_py:518:generate_setup_py(GenerateSetupPyRequest) -> GeneratedSetupPy, gets=[Get(PyDigest, CreateDigest)]). All potential sources were eliminated: []
  No source of dependency Get(ResolvedPythonDistributionEntryPoints, ResolvePythonDistributionEntryPointsRequest) for @rule(pants.backend.python.goals.setup_py:534:generate_setup_py_kwargs(GenerateSetupPyRequest) -> FinalizedSetupKwargs, gets=[Get(ExportedTargetRequirements, DependencyOwner), Get(SetupKwargs, ExportedTarget), Get(ResolvedPythonDistributionEntryPoints, ResolvePythonDistributionEntryPointsRequest)]). All potential sources were eliminated: []
  No source of dependency Get(SetupKwargs, ExportedTarget) for @rule(pants.backend.python.goals.setup_py:534:generate_setup_py_kwargs(GenerateSetupPyRequest) -> FinalizedSetupKwargs, gets=[Get(ExportedTargetRequirements, DependencyOwner), Get(SetupKwargs, ExportedTarget), Get(ResolvedPythonDistributionEntryPoints, ResolvePythonDistributionEntryPointsRequest)]). All potential sources were eliminated: []
Also understandable, as I think there is a chain of linked deps that just got broken by my hack - so, it's more a matter of not knowing how to solve THAT problem....
b
I'm not sure, but I don't think a union is the right approach here. https://www.pantsbuild.org/docs/rules-api-unions
Might be easier to fork Pants and do edits in a branch you can publish 😉
w
I was trying to vibe off these docs: https://www.pantsbuild.org/docs/plugins-setup-py#1-set-up-a-subclass-of-setupkwargsrequest - but SetupKwargsRequest is a defined external API hook, so it has the skeleton it requires. Trying to just randomly inject code won't work as well for me unfortunately 🙂
b
oh huh look at that
Yeah looks like that'll allow you the kwargs part, but not the import part
OK I think I understand now. It should be trivial to add new code to Pants to allow you to also customize the imports in addition to the args
I think for 
mypyc
to be there it needs to be a (transitive) dependency in the target. I think your custom plugin code should be able to slip that in
w
So, my target is functionally just 
python_distribution
renamed to 
mypyc_python_distribution
- so we get all the fields already for making a wheel, including ext_modules which already exists. To gather the source files, I use transitive dependencies to see what we have and add that to the mypycify function. I guess the interesting question is whether what I'm trying to achieve can be done strictly via a plugin - without touching mainline pants code. I have my doubts, but it's interesting to try to make it happen!
b
At the very least Pants will have to be touched for the import, right?
Unless you want to do like 
__import__
shenanigans 🤮
h
I think you'd have to author your own setup.py files, or customize the auto-generation
You can specify custom kwargs, if I remember correctly
in the BUILD target
but you can't add imports
Possibly what we want is to allow you to customize that entire template
👍 2
Make it an option value instead of a hard-coded string
That should work I think?
h
Or expose a plugin hook to let you customize the whole template? Right now we only have a plugin hook to change what kwargs go into the 
setup()
function call itself
b
Yeah ok pants maintainers have the better option 😁
w
@happy-kitchen-89482 @hundreds-father-404 That's kinda what I was hoping we had somewhere. Some sort of functionality to pass in a template, or override the 
generate_setup_py
function. I was also looking at this for a cython project I have. Since it's so few files, I hand create the setup.py, not a big deal - but thinking bigger picture, it would be cool to have the option to pass in all my files, or a subset (situationally)
For my repo, I hand-create the setup.py and run mypyc over it, then pass it into Pyoxidizer - works great, but hand-maintaining a setup.py (or I guess I could glob it).
h
A plugin hook would be the most dynamic option
@wide-midnight-78598 this isn't holding you up, right?
w
Well, depends. I'm able to use mypyc by creating a setup.py, so no. But, I was hoping to create a plugin tonight that would handle this all for me. Just getting back into it now - so I was looking around where I could add the functionality that sorta mimicks how the SetupKwargsRequest works
I'll gladly take a crack at this, just not sure of the right path
h
@hundreds-father-404 any pointers on how to create a plugin hook? I think it would look similar to the SetupKwargs machinery
w
I basically copied something like SetupKwargs, but the Union is where I fall over. any change I make appears to fail in like 10 Union errors, which is where I stopped earlier today. I just don't think I understand well enough how the rule dependency tree is created yet. I have a custom GenerateSetupPyRequest (or whatever), I see in the unionmapping that there are two, I even made a similar 
determine_generate_setup_request
, everything looks good - but the place where I get stuck is the Rule, or Union, or mechanism that allows overriding the original generate_setup_py function
If I remove the original, predictably everything explodes. But when I add the new one, it still complains about either there being two, or that it can't find any matching rules. Like this:
Copy code
No installed rules return the type GenerateSetupPyRequest, and it was not provided by potential callers of @rule(pants.backend.python.goals.setup_py:518:generate_setup_py(GenerateSetupPyRequest) -> GeneratedSetupPy, gets=[Get(PyDigest, CreateDigest)]).
    If that type should be computed by a rule, ensure that that rule is installed.
    If it should be provided by a caller, ensure that it is included in any relevant Query or Get.
The reason it's weird is because that happens after I ADD a new function similar to 
determine_setup_kwargs
h
This was how I added the hook for SetupKwargs: https://github.com/pantsbuild/pants/pull/10721 Taking a step back, thoughts on how the two plugin hooks would interact? You could use your new hook that entirely controls how the setup.py gets generated...or use the more limited SetupKwargs one to keep Pants setting up the kwargs, but with your plugin's inserted into it
w
I was wondering the same thing. I'm currently JUST trying to hook into the generated setup py file - and worry about the rest later, but there is something I'm just not grokking here in terms of the level of indirection.
h
Would it help to video chat for a little tomorrow or friday? might be easier than doing over slack
w
Yeah, that would be great! I'll keep plugging away until then - I can tell I'm very close, just one construct I'm missing somewhere
❤️ 1
h
I'm not sure if this was already written when you were first learning the rule graph, but making sure you have seen https://www.pantsbuild.org/docs/rules-api-tips#debugging-rule-graph-issues (And apologies in general, we know rule graph issues are confusing. We really want to improve them, only tricky with prioritization and the prereq work necessary)
w
Thanks! And yep, read all that - exporting the graphviz files, etc
Alright, I think I'm getting somewhere - this code works, but I'm not proposing it as a solution because it's a bit short-sighted right now. I just basically copied how you made the SetupKwargsRequest functionality, replicated it to perform similar on the setup.py content, and then made a custom SetupPyContentRequest for my mypyc rule. https://github.com/sureshjoshi/pants-plugins/blob/2-mypyc-prototype/pants-plugins/experimental/mypyc/rules.py https://github.com/sureshjoshi/pants/commit/9f9cbe01d9da18a800e0619fea15d29514986de9 Unfortunately, my 
pants_from_sources
isn't running correctly yet, so I need to debug that a little - but this setup was running when I was editing the cached pants 2.9.0 on my system. Thanks for all the help @hundreds-father-404!
🙌 2
👀 1
b
Where is 
mypyc
expected to come from? Provided by the backend or in the user's reqs?
Nevermind I see it in 
pants.toml
[setuptools
🙌
w
Yep, sorry, I haven't added a Readme just yet - was in the process of closing down my pyoxidizer repo now that my PR is mainline 🙂
b
I just got excited and wanted to try it out :P
Hmm I'm not seeing my transitive deps in the chroot. But I don't think you touched that code 🤔
But I do see the transitive deps listed in the call to 
mypyc
w
Hmm, strange. Note, I was testing on 2.9.0 - haven't tried anything more recent yet. Also, I've only tested the hellofib project, so there are probably a world of problems. Going to start testing on other repos soon
b
I'm testing on Pants itself 🐶 🥫
Definitely not you, when I change to use 
python_distribution
I see the same behavior
Yeah I had bad config. Attempt #2
So far so good, had to make a handful of 
__init__.py
files to make 
mypy
happy
OK so lots of 
error: Library stubs not installed for
🙂
w
Yeah, I anticipate lots of errors - another option to add to mypyc is allowing a subset of files, but at that point, it's probably the same effort to just create a custom setup.py - unless using a new python_sources target, and have mypyc operate on only those.
What I found interesting is that I wrapped up hellofib in pyoxidizer and it's much slower to run vs calling against compiled python objs - like, I think 2x slower for the actual Fib part
b
One thing I see maybe, is that the distribution should include third party deps, right?
I think that's part of the problem. When runnning 
mypyc
, 
mypy
isn't being run an environment that has those installed
w
Like, when calling from setuptools? mypy is installed via the pants.toml - and the mypy package is what holds mypyc Aside: https://github.com/sureshjoshi/pants-plugins/blob/2-mypyc-prototype/pants-plugins/experimental/mypyc/README.md
Ohhh, okay, sorry, I understand
Yeah - right now it's source only, but I need to pull in some deps to test it out properly. That'll be part of those other hello-* subprojects, a small testbed to mess around
b
To hack it, I'm just gonna copy all the deps
w
I'll have more to report when I can get pants_from_sources working, unfortunately a no go just yet
b
Copy code
pants/backend/python/pip_requirement.py:13: AssertionError:
With no output FFS
w
Which branch are you on of pants?
b
main
w
whoa
b
Lots of internal 
mypyc
assertions.
oof. So if you have an untyped property -> Empty AssertionError
Probably need to run 
mypy
on pants in a more strict mode to catch these early
👍 1
w
Only if it can't figure out by itself
b
What's weird is my editor could (although it uses 
pyright
)
w
So, its funny, pylance/pyright (or whatever it's called now) has a hard time for me in the pants repo. Lots of red, even with all the libs I expect. Mypy doesn't have as much trouble
b
Thats likely because the Pants repo has a 
mypy
config, but not a 
pyright
one
chrisjrn added an 
env
file which clues VS Code into Pants' repo structure. SO 
pyright
works
Found 1369 errors in 503 files (checked 916 source files)
oh shit
mostly test files 😕
OK cmon 
mypy
that's just goofy:
Copy code
@property
    def message_path(self):
        return f"{self.path_str}.lock_message"
You really can't deduce the return type?
w
I wonder if there's an auto-type added
❗ 1
*adder
b
Upgrading mypy seems to help my issue
Although this is a pain in my ass now: https://github.com/python/mypy/issues/5144
w
which version of mypy are you on?
b
oh hell, after fixing all the 
mypy 0.930
issues, I still hit the same assert as on 
0.910
There went an hour for nothing 😠
I suppose I could make a PR to upgrade Pants' repo to 
mypy
0.930
now 🙄
❤️ 2
w
Is there a slow subset of code it can be tested on? See if/how much improvement there is? I'm going to be doing some web server testing - but I think FastApi uses a whole lot of "magic" code with pydantic, so I'm skeptical it'll work
h
I suppose I could make a PR to upgrade Pants' repo to mypy 0.930 now
Yes please! I wanted to upgrade last week but reverted it because all the issues
b
OK @hundreds-father-404 will do at some point 😄
❤️ 1
w
This "Cannot find implementation or library stub for module" is pretty strange, since python_distributions should be packaged with those
Also, turns out pyannotate does handle adding typings 🙂
🙌 1
b
@wide-midnight-78598 Wouldn't they simply show up in 
install_requires
, but not necessarily be packaged with them?
w
Hmm, good point. I could swear I saw them somewhere when I was running through this, but maybe I actually just saw references to them
b
But also I've hit my limit on doing this to pantsbuild/pants.
Copy code
File "mypy/nodes.py", line 2678, in __getitem__
pants/init/logging.py:111: KeyError: 'trace'
I guess 
mypyc
doesn't care about 
#type: ignore[attr-defined]
w
Mayeb doesn't like them?
b
Who even reads docs anyways? 😅
OK noooow I'm done:
Copy code
File "mypyc/irbuild/context.py", line 128, in curr_env_reg
pants/core/goals/lint.py:92: AssertionError:
w
Pretty big challenge for a first pass at using this :)
b
I was naive
Also, it doesn't help I get one error at a time. I keep wondering if this one will be the last one 😅
Copy code
pants/backend/python/lint/bandit/rules.py:23: KeyError: "'BanditRequest' has no attribute 'prior_formatter_result'"
That's odd, because that attr is 100% in the class' hierarchy
OK you reeeealy need to set 
--disallow-untyped-defs
that seems the easier route, lol
👍 1
How good is 
pyannotate
? 🤔
@hundreds-father-404 how well-received would a PR to the pants repo be with a type-blast
w
I just saw mypyc recommend PyAnnotate (dropbox) and MonkeyType (instagram) - maybe module by module?
h
OK you reeeealy need to set --disallow-untyped-defs that seems the easier route, lol
I am ALL for making MyPy stricter in Pants. I'd love to do that. I only first started adding MyPy in 2020 iirc, so we still have some untyped code
how well-received would a PR to the pants repo be with a type-blast
Well, but encouraged to do in several smaller PRs to make it easier to review
w
Like, I'd be curious if it works just on the plugins (to start) and then work out/in
b
Here's how far I got: https://github.com/thejcannon/pants/tree/mypyc Looks like 
mypyc
doesn't like functions returning instances of inner-classes
w
is it because of late-binding closure type things?
b
🤷‍♂️ my next attempt would be to just use the type constructor
But I've neglected my day job long enough 😬
👋 2
5 Views
Open in Slack
PreviousNext
Powered by