Replies: 4 comments 1 reply
-
|
Hi @edreamleo, thank you for your interest in rope! This is awesome. This PR is massive, I'll have to take a look at it tomorrow, but while I think there are some good ideas here, I think it is likely that some parts will need to be split off into more reviewable logical PRs as even PR#3 is still quite big. Here's a quick few things I noticed:
That's it for now, but I'll take a look into this again tomorrow. Again, thanks for the interest to help with rope. You don't have to answer this immediately, but once you're more settled with the codebase, do you have any particular features/areas that you are interested in? |
Beta Was this translation helpful? Give feedback.
-
|
On Thu, Nov 24, 2022 at 12:42 PM Lie Ryan ***@***.***> wrote:
Hi @edreamleo <https://github.com/edreamleo>, thank you for your interest
in rope! This is awesome.
Thanks. I think rope is awesome too :-)
This PR is massive, I'll have to take a look at it tomorrow, but while I
think there are some good ideas here, I think it is likely that some parts
will need to be split off into more reviewable logical PRs as even PR#3 is
still quite big.
That's completely fine. Your comments suffice in many ways.
I just wanted to start the conversation. I agree that the PR should be
split. Indeed, it would be natural to create multiple rope issues first.
Here's a quick few things I noticed:
-
I would suggest removing the pynames/pynamesdef reorganisation. The
pynames and pynamesdef distinction is actually quite important, even if it
may not be behaviorally necessary to distinguish them. If anything, I'd
probably actually make those modules have proper duplicates with
inheritance rather than some classes just being star imported from the
other.
Good. I wasn't sure that the "hack" of moving the pynamesdef methods into
pynames made sense. Otoh, the statement `import pynamesdef as pynames` in
pyobjectnames is hard to understand. As you say, using inheritance would
likely be easier to understand. For one thing, the subclass's docstring
could discuss why the subclass is desirable :-)
- I've not been a fan of the ast name either. However, I would prefer
not to do an as-rename import astutils as au though, two letter module
names might save some typing but IMO when reading through larger codebase,
it makes the code much harder to follow to have to mentally keep track of
the abbreviations. Just use the astutils.parse() for the qualifiers.
I agree. As I implied in the first comment of #3, astutils.parse,
astutils.walk would be fine. There are even precedents in rope's present
code for having no qualifier at all for walk, parse, etc.
- I agree that the imports should be simplified to just one level. from
rope.base import xxx should be the norm for most imports rather than
the fully qualified names like import rope.base.xxx
Thanks for this comment. I wasn't completely sure whether there might be
some obscure reason for "over-qualified" names.
- the unittest2 cleanup, actually I happened to already do this
cleanup in latest master for rope==1.5.0 release, this was 3 days ago, so
if you sync/rebase your branch they should no longer exist.
Oh good.
Again, thanks for the interest to help with rope.
You're welcome. I would appreciate your suggestions for how to move to rope
1.0. Maybe there should be a sequence of rope issues, each with its own PR.
And perhaps it will soon be time for a rope-1.0-dev branch for all 1.0
work. Of course, this is all your call.
You don't have to answer this immediately, but once you're more settled
with the codebase, do you have any particular features/areas that you are
interested in?
Yes. First, I would like to understand rope's type inference in complete
detail. My first PRs were meant to make me comfortable with studying rope's
code. I look forward to being able to ask you more informed questions :-)
Second, it may be time to consider mypy-based type inference. Something
akin to rope #146 <#146>, which
was proposed in 2016. Iirc, mypy did not exist then. Now, mypy is a mature
project, with many heavy hitters among its devs.
So in addition to the (expensive!) doi and soi modules, it might be
possible to add a similar aoi (annotation-based inference) module to rope.
Something like this:
1. A prepass would determine the *final* types of all *declared *objects,
based on python 3 annotations. (There is no need to support type comments.)
Most of rope's annotation machinery would likely still exist, but this
machinery will be local, and so very fast. (An alternate name for aoi would
be loi: local object inference.)
For example, annotations only tell us the name of function arguments and
function returns. aoi would determine the types of vars, within
functions/methods or at the outer level of modules.
2. aoi might interpret 'Any' (or maybe even "bare/general" Dict, List,
Tuple, etc) pragmatically. 'Any' could mean something like "any object/type
declared with the program being run". This might be particularly helpful
for typing completion.
Whatever the merits of 2, aoi promises to speed up rope a lot.
3. I am a fan of annotations. I would like to add complete annotations for
all of rope.base and rope.oi.
*Summary*
We both agree that PRs 3, 4, and 5 are expendable. They have served their
purpose: getting me comfortable with working with rope's code.
I'll defer to your suggestions about the issues and PRs needed to get to
rope 1.0. Your comments about my original post are plenty good enough for
now.
I have the following aims:
- Help rope get to 1.0:
- Eliminate python2-isms and related cruft.
- No complaints from mypy, pylint, and pyflakes.
- Add annotations for everything in rope.base.
- Completely understand rope's type inference.
- Add aoi (annotation-based inference).
Edward
P.S. Perhaps I should say a few more words about myself.
1. Many years ago (before mypy) I tried my hand at creating a type checker.
This work failed spectacularly. However, it did give me an understanding of
the shape of the overall type-checking problem. I am awe-struck with mypy's
code. It goes way beyond what I ever imagined.
2. I understand python's ast module very well. See the link to leoAst.py
<https://leoeditor.com/appendices.html#leoast-py> at the bottom of python's
for ast.py page.
3. Type inference, and particularly the Hindley-Milner algorithm, still
seems mysterious. Indeed, the so-called "algorithm" seems more like a
template. Perhaps the algorithm really just defines what the "best" type
means.
4. I recently added type annotations throughout Leo. It was a big job, but
well worthwhile. mypy's gradual typing works superbly for large projects
such as Leo or rope.
Edward
|
Beta Was this translation helpful? Give feedback.
-
|
That is awesome. Note that rope already has a 1.0.0 version, the delimiting goal for version 1.0.0 back then was completing support of Python 2.x syntaxes and dropping support for Python 2.x. This is already pretty much done, though as you noticed, there are still many Python 2-isms left around that can be cleaned up. The next step for rope currently is supporting new syntaxes from Python 3.11+ and better support for type hints (especially string lazy annotations). We haven't supported exception groups and
Yeah, I agree, that import alias is definitely a wtf.
Type inference is a part of rope that's currently good enough to be decent but also somewhat underdeveloped as much of it hasn't really keep up with PEP 484 annotations, so this is great. Tbh, I am not really familiar much with the current type inference mechanics myself. If you have any questions, I help as much as I can but many of these parts won't be familiar to me either.
That is a very interesting idea. It seems like it'll be a significant undertaking, but I think it's worth looking into. Most of the implementations for the current type inference is in I never actualy used doi, so I don't even know how to actually use that or whether it even still works. I am doubtful that doi is a good idea, and since it's disabled by default and I almost never saw anyone reporting bugs about it, I am doubtful that anyone's even actually using doi. It's been on my list of things to potentially just remove from rope.
From the sound of it, you seem to have a lot of experience with type checking/inference and its internal, so when it comes down to it I will trust your judgements on them. My use of type annotation/mypy has generally been quite basic. I tend to annotate more judiciously than most people; generally, only when it's not immediately obvious what the types should be or when the type checker can help with enforcing some important invariants. I find that over-annotations can often make it harder to actually understand the actual logic of the code, as the meat of the code can be easily lost among the annotations, so I'd never been a fan of complete annotation of the entire codebase. This is the kind of (admittedly, extreme and artificial) example of code that makes me grumpy: as it has a very high signal to noise ratio, and the annotations are actually harming readability compared to the unannotated code: But I think this is where gradual typing are really great, because it gives you the flexibility to type as much or as little as needed to be useful rather than as an exercise to annotate everything. Rope certainly can benefit from more annotations, there are many parts of rope's codebase where type hints can really help disambiguate between similar concepts that shouldn't be interchangeable, and it can take you a good hour trying to figure out what variables actually should contain, but I don't recommend making annotating everything to be a goal.
I do have my own set of priorities of what I consider important. rope is a community project, so I do not set anyone's priorities; you should work on things that matters to you or interests you the most :) For me, my own priority is on core refactoring operations (i.e. things in |
Beta Was this translation helpful? Give feedback.
-
|
On Fri, Nov 25, 2022 at 10:22 AM Lie Ryan ***@***.***> wrote:
That is awesome.
Glad you approve :-)
Note that rope already has a 1.0.0 version, the delimiting goal for
version 1.0.0 back then was completing support of Python 2.x syntaxes and
dropping support for Python 2.x. This is already pretty much done, though
as you noticed, there are still many Python 2-isms left around that can be
cleaned up.
Good to know.
The next step for rope currently is supporting new syntaxes from Python
3.11+ and better support for type hints (especially string lazy
annotations). We haven't supported exception groups and except* syntax,
which is likely what I am looking into next.
Otoh, the statement import pynamesdef as pynames in pyobjectnames is hard
to understand.
Yeah, I agree, that import alias is definitely a wtf.
Yes. First, I would like to understand rope's type inference in complete
detail.
Type inference is a part of rope that's currently good enough to be decent
but also somewhat underdeveloped as much of it hasn't really keep up with
PEP 484 annotations, so this is great. Tbh, I am not really familiar much
with the current type inference mechanics myself. If you have any
questions, I help as much as I can but many of these parts won't be
familiar to me either.
Thanks. I'll begin deeper studies of Ropes inference soon.
Second, it may be time to consider mypy-based type inference.
That is a *very* interesting idea. It seems like it'll be a significant
undertaking, but I think it's worth looking into.
Glad you are interested. This is a long term project, presumably
significantly after 1.0.
Most of the implementations for the current type inference is in
rope/base/oi/ and much of it was implemented by @emacsway
<https://github.com/emacsway> a long time ago. From my understanding, the
soi implementation has a pluggable interface for plugging in different
inference mechanisms; I don't know whether the existing pluggable interface
will be adaptable enough to adapt to mypy's own inference engine or
whether you might need to design a new interface for this, as I'm familiar
with neither the current soi implementation nor mypy's internal workings.
I never actualy used doi, so I don't even know how to actually use that or
whether it even still works. I am doubtful that doi is a good idea, and
since it's disabled by default and I almost never saw anyone reporting bugs
about it, I am doubtful that anyone's even actually using doi. It's been on
my list of things to potentially just remove from rope.
Thanks for this high quality information. I would not have guessed any of
this from the code itself.
1. Many years ago (before mypy) I tried my hand at creating a type
checker
2. I recently added type annotations throughout Leo
From the sound of it, you seem to have a lot of experience with type
checking/inference and its internal, so when it comes down to it I will
trust your judgements on them. My use of type annotation/mypy has generally
been quite basic.
To be clear, I have lots of experience with python's ast, but much about
type inference remains a mystery. Sure, local deductions are
straightforward, but knitting together the local deductions is anything but
straightforward. So I have a lot to learn.
I tend to annotate more judiciously than most people; generally, only when
it's not immediately obvious what the types should be or when the type
checker can help with enforcing some important invariants. I find that
over-annotations can often make it harder to actually understand the actual
logic of the code, as the meat of the code can be easily lost among the
annotations, so I'd never been a fan of complete annotation of the entire
codebase. This is the kind of (admittedly, extreme and artificial) example
of code that makes me grumpy:
project: Optional[projects.Project] = cast(Optional[projects.Project], projects.Projects.from_path(project_root))
if isinstance(project, projects.Project):
...
as it has a very high signal to noise ratio, and the annotations are
actually harming readability compared to the unannotated code:
project = projects.Project.from_path(project_root)
if project is not None:
...
Yes, we want to annotate the invariants first.
Abbreviations can help a lot. And with the right settings Optional is
optional :-) So your example might be rendered something like this:
project: Project = projects.Projects.from_path(project_root)
if project is not None:
In fact, given a reasonable annotation of Projects.from_path, mypy might
infer the type of project without any help:
project = projects.Projects.from_path(project_root)
if project is not None:
And anyway, gradual typing always allows this as an interim measure:
project: Any = projects.Projects.from_path(project_root)
if project is not None:
In short, my experience with recent versions of mypy is completely
positive. Eventually I'll take a crack at annotating Rope and see what
happens.
But I think this is where gradual typing are really great, because it
gives you the flexibility to type as much or as little as needed to be
useful rather than as an exercise to annotate everything. Rope certainly
can benefit from more annotations, there are many parts of rope's codebase
where type hints can really help disambiguate between similar concepts that
shouldn't be interchangeable, and it can take you a good hour trying to
figure out what variables actually should contain, but I don't recommend
making annotating everything to be a goal.
Excellent.
------------------------------
I'll defer to your suggestions about the issues and PRs needed to get to
rope 1.0.
I do have my own set of priorities of what I consider important. rope is a
community project, so I do not set anyone's priorities; you should work on
things that matters to you or interests you the most :)
For me, my own priority is on core refactoring operations (i.e. things in
rope.refactor). IMO, this is the part where rope is the most advanced
compared to other similar tools in the ecosystem, it is rope's killer
feature. Other parts like go-to-definition and codeassist/completion have
other great python implementations, like jedi, so they hadn't been very
high priority for me to improve. That said, the other parts of rope can be
useful to get people to install rope, most people use code completions and
go-to-definition all the time, but automated refactoring tools is something
that is less frequently needed. But if rope is already on their system
because they already using rope completion, that is one less barrier of
entry for users to start trying out rope's refactoring.
Thanks for this. I wonder whether a separate issue or discussion would help
coordinate our plans. Maybe a zoom session or two would help. Are you
interested?
Edward
|
Beta Was this translation helpful? Give feedback.
-
|
@lieryan It's been an exciting week. Here is a progress report. I've come up to speed with your style of PRs and management. I've learned a few things about git. I'm amazed that I have found such excitement in a subordinate role in a project not my own. It's challenging to have to convince you that my ideas make sense. It's reassuring that I can't do anything stupid on my own, hehe. My PRs are getting shorter, as I knew they would. I'll bet you are relived :-) There are several PRs on the table that suggest various cleanups, but I've just created a STUDY branch, based on master, that has things set up as I like. It doesn't include the ast to rast renaming, but that's no big deal. On to the study of Rope's type inference! I'll start by grokking (single-stepping through) a few unit tests. |
Beta Was this translation helpful? Give feedback.
-
As my profile indicates, I am the author of Leo. I've been working on the python version of Leo since 2001, but now that work is winding down. Félix Malboeuf is Leo's new "heir". He has created leoInteg, a vs-code plugin. He is now working on a pure JS version of leoInteg called leoJS. vs-code has millions of users compared to Leo's thousands, so Félix's work is the new main line of the Leonine world.
I could not be happy without programing in python, so I am looking for an interesting new project. I think my interests and experience could help create rope 1.0.
Here is my (provisional!) to-do list for rope:
@deprecateddecorators.This list is long, but I have lots of experience in this area. I enjoy such work, and Leo's cff (clone-find) commands are powerful tools at my disposal.
Later, I would like to experiment with having rope make type inferences based primarily on mypy annotations.
About the PRs
As stated in each PR, the PRs are prototypes. PRs 3, 4, and 5 form a "chain": PR 4 is based on PR 3, and PR 5 is based on PR4. This organization is (temporarily!) convenient for me. I am willing to do whatever is needed to fold the prototype code into (say) a rope-1.0 branch. All comments and suggestions are welcome!
Beta Was this translation helpful? Give feedback.
All reactions