Fix another non module root#2744
Conversation
Pierre-Sassoulas
left a comment
There was a problem hiding this comment.
Thank you for chiming in and investigating the root cause ! LGTM, just wondering if we should backport or wait for 4.0.
| def __init__( | ||
| self, | ||
| parent: NodeNG, | ||
| lineno: None = None, | ||
| col_offset: None = None, | ||
| parent: None = None, | ||
| *, |
There was a problem hiding this comment.
Should we consider this a breaking change ?
There was a problem hiding this comment.
You are right, it is.
There was a problem hiding this comment.
What is the procedure for handling this?
There was a problem hiding this comment.
We'll wait for 4.0 to release it, and in astroid we just add a mildly strongly worded changelog entry. Or maybe we're not going to, I'm sure @DanielNoord will have an opinion about it :)
| @property | ||
| def attr___annotations__(self) -> node_classes.Unkown: | ||
| return node_classes.Unknown() | ||
| def attr___annotations__(self) -> node_classes.Unknown: |
There was a problem hiding this comment.
Nice catch, strange that this typo was not a problem anywhere.
There was a problem hiding this comment.
My local checker (not mypy, pyright) found other typing issues in that file. Maybe mypy isn't running on this file for whatever reason.
There was a problem hiding this comment.
Yeah, mypy doesn't run on a lot of file at the moment. Lots of dynamic things that are very hard to type. I would have thought that actual import of something that does not exist would have been an error, but it seems it's not.
There was a problem hiding this comment.
I think it's due to from __future__ import annotations at the top. It makes python treat every annotation as a string. Without it, it would have been checked.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2744 +/- ##
==========================================
- Coverage 93.25% 93.24% -0.01%
==========================================
Files 93 93
Lines 11072 11073 +1
==========================================
Hits 10325 10325
- Misses 747 748 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
@DanielNoord release in 4.0.0 as breaking or backport in 3.3.11 ? |
|
I think back porting is fine :) |
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-maintenance/3.3.x maintenance/3.3.x
# Navigate to the new working tree
cd .worktrees/backport-maintenance/3.3.x
# Create a new branch
git switch --create backport-2744-to-maintenance/3.3.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 e7a8669fcc586228ddb3774f9b6fc7ea184b98fe
# Push it to GitHub
git push --set-upstream origin backport-2744-to-maintenance/3.3.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-maintenance/3.3.xThen, create a pull request where the |
|
Sadly the fix can't be applied on 3.3.11 because |
This fixes Unknown nodes that are created in ObjectModel without a parent. Furthermore, it forbids creation of Unknown nodes without a parent.
It also creates an xfailing test for #2743.
Type of Changes
Description
Refs #2739, #2743.
Closes #120