Add back "True-True" CI #2123
Add back "True-True" CI #2123
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
| - openeye: true | ||
| python-version: "3.11" |
There was a problem hiding this comment.
This is the primary culprit; we need to double-check the CI matrix after removing old Python versions (or run more combinations on multiple version)
| raise ValueError("The graph must be a NetworkX graph.") | ||
|
|
||
| atom_nums = list(dict(graph.nodes(data="atomic_number", default=1)).values()) | ||
| atom_nums: list[int] = [atomic_number for (_, atomic_number) in graph.nodes(data="atomic_number", default=1)] |
There was a problem hiding this comment.
This change is motivated in part by how hard it was to figure out what was going on at first; it reads like the type would be list[dict[?, ?]] but in fact the iterator in the middle returns a sequence of 2-length tuples:
In [6]: [*Molecule.from_smiles("CCO").to_networkx().nodes(data="atomic_number", default=1)]
Out[6]: [(0, 6), (1, 6), (2, 8), (3, 1), (4, 1), (5, 1), (6, 1), (7, 1), (8, 1)]of which we only care about the second in each. Casting to dict and then only getting the values is an okay way to extract the numbers we want, but I think iterating over the tuples directly and throwing away the first value is more explicit (can use a helpful variable name atomic_number inside) and direct (skips a step)
| offmol.add_default_hierarchy_schemes() | ||
| offmol.add_default_hierarchy_schemes() # type: ignore[operator] |
There was a problem hiding this comment.
This method if properly defined, but something in the writing was getting crossed and add_default_hierarchy_schemes was inferred to be a list[HierarchyElement]. That's not a function, so ()-ing it is an error
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
| - nglview | ||
| - parmed =4 | ||
| - mypy =1.15 | ||
| - mypy =1.18 |
There was a problem hiding this comment.
This is the most recent version; if there's a reason to downpin I am happy to update
| def angles(self) -> Generator[tuple["Atom", ...], None, None]: | ||
| def angles(self) -> Generator[tuple[AtomLike, AtomLike, AtomLike], None, None]: |
There was a problem hiding this comment.
This changes are intentional
- Having it cover
Atom | _SimpleAtomis intentional - Being length-3 (length-4 below) is important;
tuple[x, ...]can be a tuple of any length but containing onlyX(docs)
Resolves #2092 while attempting to be minimally invasive
This overlaps somewhat with #2121 and whichever is merged first should be accounted for in the other