Skip to content

Minor fixes#1965

Open
kxrob wants to merge 12 commits intomhammond:mainfrom
kxrob:_minor_fixes
Open

Minor fixes#1965
kxrob wants to merge 12 commits intomhammond:mainfrom
kxrob:_minor_fixes

Conversation

@kxrob
Copy link
Copy Markdown
Collaborator

@kxrob kxrob commented Nov 4, 2022

No description provided.

Property 'dispid's only found by __AttrToID__() / GetIDsOfNames()
were not cached in propMap and caused time consuming search upon
every read.
@mhammond
Copy link
Copy Markdown
Owner

Thanks. Any chance you could split this into one with no functional changes and one where the behavior is changed? Some of the COM ones I'd need to dig deeper in to but some are 100% cosmetic.

and remove __cmp__ for Python3. __eq__, __lt__ are already
defined in base class HLIPythonObject.
Undefined name 'argparse' in verify_destination()
Search `path + os.pathsep`, not `os.pathsep + path`.
(Caused duplicate entry when already at the beginning)
`disp.__dict__.get("CLSID")` was always None. Triggered
repeated search and recapture every time.
@kxrob
Copy link
Copy Markdown
Collaborator Author

kxrob commented Nov 16, 2022

Removed the changes with no functional effect (mainly unused imports and variables)

@mhammond
Copy link
Copy Markdown
Owner

Thanks - but I was hoping the changes without functional effect were uploaded as I'd land them - eg, the freeing of a variable isn't controversial so I'd be fine landing that (as well as unused imports which you said you removed). But the ones like the removal of __cmp__ and changing of __nonzero__ to __bool__ might make sense, but I want to tread carefully - eg, the __bool__ one does follow the intent and functionality back in the 2.X days, but now does run the risk of breaking code that's been working in 3.X for many years.

@kxrob
Copy link
Copy Markdown
Collaborator Author

kxrob commented Nov 18, 2022

changes without functional effect were uploaded

I've parked the few de-lints in a new branch, but think its too small and low priority for a PR and will push occasionally with more. (It mainly disturbed getting a clean lint status on edited files - new unused stuff is often an error)

removal of __cmp__

__cmp__ is never used in Py3.
The base class HLIPythonObject already provides the appropriate __lt__ and __eq__ regarding self.name, which is enough to support sort in the tree display context.
( @functools.total_ordering would provide full comparisons but not used ... )

changing of __nonzero__ to __bool__

dynamic.CDispatch.__bool__ already exists (via 2to3 2316aef) regarding the same purpose "This class has a __len__ - this is needed so 'if object:' always returns TRUE."
It created a problem here where a COM object by chance had a __len__ and zero .Count with surprising behavior under certain circumstances. One indeed doesn't expect a non-true for an existing COM object in if comobj:.
So I think overall its more sane to see it as bug - remaining, it would rather add to the incompatibilites with dynamic ( #1943 , #1931 ...) and 2to3s and give raise to more new and lingering surprises vs. very rare chances sb already relied on an odd length check of generated vs dynamic class.

@Avasam
Copy link
Copy Markdown
Collaborator

Avasam commented Apr 14, 2024

@kxrob A handful of these changes have been included as part of old code refactoring I've been doing to prepare for type annotations. I think that if you resolve conflicts you'll find this PR to be even smaller than before :)

retEntry = self._olerepr_.propMapGet.get(attr)
if retEntry is None:
retEntry = build.MapEntry(self.__AttrToID__(attr), (attr,))
self._olerepr_.propMap[attr] = retEntry
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I've extracted this change to #2750

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