Skip to content

Use variable name to determined NamedTuple class name #6698

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

Merged
merged 1 commit into from
May 28, 2020
Merged

Conversation

msullivan
Copy link
Collaborator

This lets us avoid inserting line numbers into the name when the
variable name and argument to NamedTuple disagree. This is good,
because the line numbers are ugly and when combined with bug #6548
causes problems when a namedtuple is reexported.

@msullivan msullivan requested a review from ilevkivskyi April 18, 2019 21:15
@ilevkivskyi
Copy link
Member

As we discussed offline, it may be better to do this as part of a larger clean-up proposed in #6422, unless this is urgent.

@msullivan
Copy link
Collaborator Author

This issue got reported again at Dropbox and since #6422 seems moribund, I've rebased this. I don't remember what the objections we discussed were. @ilevkivskyi, what are your thoughts about merging this now?

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 16, 2020

I didn't do a full review, but I think that this is a reasonable incremental improvement. I agree that it would be even better to fix #6422, but it could take a long time before it happens, unfortunately.

@ilevkivskyi
Copy link
Member

@msullivan Yes, go ahead, it is unlikely I will ever work on #6422.

This lets us avoid inserting line numbers into the name when the
variable name and argument to NamedTuple disagree. This is good,
because the line numbers are ugly and when combined with bug #6548
causes problems when a namedtuple is reexported.
@msullivan
Copy link
Collaborator Author

Apparently after rebasing this I then forgot about it, so I'm rebasing again and am actually going to land it.

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.

3 participants