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

Separate pynames and pynamesdef and remove star-import #618

Merged
merged 2 commits into from Dec 19, 2022

Conversation

lieryan
Copy link
Member

@lieryan lieryan commented Dec 19, 2022

No description provided.

@lieryan lieryan self-assigned this Dec 19, 2022
@lieryan
Copy link
Member Author

lieryan commented Dec 19, 2022

@edreamleo this is my proposal if you really cannot have the import *

I'll expand further on why the #587 and related changes doesn't really make sense as a comment on that PR.

@edreamleo
Copy link
Contributor

@lieryan Hurray! This works for me. Many thanks.

Copy link
Contributor

@edreamleo edreamleo left a comment

Choose a reason for hiding this comment

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

Many thanks for meeting me half way.

@lieryan
Copy link
Member Author

lieryan commented Dec 19, 2022

Thanks for confirming that! Glad to find a common ground that we can both settle for.

While this has been a hard argument, but I think it is very worthwhile to think about these things and to always keep challenging and reevaluating incumbent design decisions. Thank you for making me have to think! It's a good kind of headache to have 😀.

@lieryan lieryan merged commit d6ea864 into master Dec 19, 2022
@lieryan lieryan deleted the lieryan-pyobjects branch December 19, 2022 15:07
@lieryan lieryan added this to the 1.7.0 milestone Dec 19, 2022
@edreamleo
Copy link
Contributor

@lieryan

While this has been a hard argument, but I think it is very worthwhile to think about these things and to always keep challenging and reevaluating incumbent design decisions. Thank you for making me have to think! It's a good kind of headache to have 😀.

You're welcome! You are teaching me about collaboration from another point of view. These are important lessons for me.

@edreamleo
Copy link
Contributor

@lieryan The usual OO pattern is to derive, say, classes 'Cat' and 'Dog' from class 'Animal'.

But this PR continues to derive (for example) rope.base.pynamesdef.ParameterName from rope.base.pynames.ParameterName. There are other examples.

As I said in another recent comment, I'm not sure what the names of the base class should be, but it seems to me that Rope's code would be clearer if the names of base classes didn't class with the names of subclasses.

To repeat, the docstrings for the base classes could clarify what's going on.

@lieryan lieryan changed the title Removing star-import Separate pynames and pynamesdef and remove star-import Jan 17, 2023
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.

None yet

2 participants