From 40890983d8cbace197d93eacf0688873945a7634 Mon Sep 17 00:00:00 2001 From: Josh Mitchell Date: Wed, 20 Sep 2023 18:29:33 +1000 Subject: [PATCH 01/28] Switch to OFE theme and remove redundant CSS --- docs/_sass/custom-api.scss | 384 ------------------------------------ docs/_static/css/custom.css | 249 ----------------------- docs/conf.py | 2 +- docs/environment.yaml | 4 +- 4 files changed, 3 insertions(+), 636 deletions(-) delete mode 100644 docs/_sass/custom-api.scss delete mode 100644 docs/_static/css/custom.css diff --git a/docs/_sass/custom-api.scss b/docs/_sass/custom-api.scss deleted file mode 100644 index 461e36e84..000000000 --- a/docs/_sass/custom-api.scss +++ /dev/null @@ -1,384 +0,0 @@ -// The Numpydoc API layout - -html { - --ofe-api-name-color: var(--ofe-color-FeelingSpicy); - --ofe-api-property-color: var(--pst-color-text-muted); - --ofe-api-path-color: var(--pst-color-text-base); - --ofe-api-bg-color: var(--pst-color-on-background); - --ofe-api-param-symbol-color: var(--pst-color-text-base); - --ofe-api-param-name-color: var(--pst-color-text-base); - --ofe-api-param-value-color: var(--pst-color-text-base); - --ofe-api-type-color: var(--pst-color-text-base); - --ofe-api-type-link-color: var(--pst-color-link); - --ofe-api-source-link-color: var(--pst-color-link); - - --ofe-api-header-font-size: 1.1rem; - --ofe-api-header-padding: var(--ofe-api-header-font-size); - --ofe-api-arguments-indent: calc(2 * var(--ofe-api-header-padding)); -} - -// Definition of an object. Hopefully. -// Could also use dl.py, but this would fail -// if we ever wanted to document something that -// isn't Python -// RTD does it like this, but with fewer exceptions, so -// we should be OK -dl:not(.docutils):not(.field-list):not(.simple):not(.citation):not(.option-list):not(.footnote)[class] { - padding-bottom: 0.5em; - border: 1px solid var(--pst-color-border); - border-radius: .25rem; - - // Text specifying class, function, method, pydantic model, etc. - // Usually present in the .property class, but if it's missing - // we can insert it - > dt { - > *:not(.property):first-child::before, > .property { - color: var(--ofe-api-property-color); - font-size: inherit; - font-weight: normal; - font-style: italic; - } - } - &.attribute > dt > *:not(.property):first-child::before { - content: "attribute "; - } - &.method > dt > *:not(.property):first-child::before { - content: "method "; - } - &.function > dt > *:not(.property):first-child::before { - content: "function "; - } - - - // Header and signature - > dt { - font-family: var(--pst-font-family-monospace); - font-size: var(--ofe-api-header-font-size); - padding: var(--ofe-api-header-padding); - background-color: var(--ofe-api-bg-color); - border-radius: .25rem 0; - // Allow words to break anywhere, if necessary - overflow-wrap: break-word; - // Position relative so we can absolutely position source link - position: relative; - &:target { - &::before { - background-color: var(--pst-color-background); - } - } - - // Indent the argument list - padding-left: calc(var(--ofe-api-header-padding) + var(--ofe-api-arguments-indent)); - > :first-child { - margin-left: calc(-1 * var(--ofe-api-arguments-indent)); - } - - // Text providing path to the object - > .sig-prename { - padding: 0; - background-color: transparent; - font-weight: 200; - font-size: inherit; - color: var(--ofe-api-path-color); - display: inline-block; - } - // Text providing the name of the object - > .sig-name { - padding: 0; - background-color: transparent; - color: var(--ofe-api-name-color); - font-weight: 600; - font-size: inherit; - // autodoc_pydantic produces types as properties _after_ the sig name - ~ .property .pre { - color: var(--ofe-api-type-color); - font-style: normal; - } - ~ .property a .pre { - color: var(--ofe-api-type-link-color); - } - - // If the source link immediately follows the name, don't position it absolutely - + a.reference.internal .viewcode-link { - position: static !important; - float: right; - margin-left: 0.5em; - } - } - // Opening and closing parenthesis - > .sig-paren { - font-size: inherit; - - } - // Each parameter - > .sig-param { - font-size: inherit; - font-style: normal; - // Entire parameter if parsing the parameter has failed. Splits on commas - > .pre { - color: var(--ofe-api-param-name-color); - } - // Name of a parameter - > .n > .pre { - color: var(--ofe-api-param-name-color); - } - // Symbols; equals sign, asterisk, etc - > .o > .pre { - color: var(--ofe-api-param-symbol-color); - padding-left: 0.2em; - padding-right: 0.2em; - } - // Type anotation - .p, .p + .n, .p + .w + .n { - font-weight: normal; - .pre { - color: var(--ofe-api-type-color); - } - a .pre { - color: var(--ofe-api-type-link-color); - } - } - // Default values of arguments - > .default_value > .pre { - color: var(--ofe-api-param-value-color); - } - // After each parameter, newline - &::before { - content: "\a"; - white-space: pre; - } - // Links - a { - &:hover { - .pre, pre, code { - color: var(--pst-color-link-hover); - } - } - } - } - // Brackets [] denoting optional arguments - // This is redundant information and I am displeased to have to support it - > .optional { - // Put optional [] brackets on their own lines - &::before { - content: "\a"; - white-space: pre; - } - // Optional parameters need extra indentation - ~ .sig-param::before { - content: "\a "; - } - } - // Closing parenthesis - .sig-param, .optional { - + .sig-paren { - &::before { - content: "\a"; - white-space: pre; - } - - // Unindent closing paren, and everything following (except source link) - position: relative; - left: calc(-1 * var(--ofe-api-arguments-indent)); - ~*:not(a.reference.internal, .headerlink) { - position: relative; - left: calc(-1 * var(--ofe-api-arguments-indent)); - max-width: calc(100% + #{var(--ofe-api-arguments-indent)} - 4em); - display: inline-block; - vertical-align: top; - } - ~ .headerlink { - position: absolute; - bottom: var(--ofe-api-header-padding); - right: var(--ofe-api-header-padding); - } - } - } - // Pydantic validator arrow - .autodoc_pydantic_validator_arrow { - &::before { - content: "\a"; - white-space: pre; - } - // Comma separating validated fields - ~ .property { - &::after { - content: "\a "; - white-space: pre; - } - } - ~ .headerlink { - position: absolute; - bottom: var(--ofe-api-header-padding); - right: var(--ofe-api-header-padding); - } - } - // Link to the source code for the object (not present on inherited objects) - .viewcode-link { - position: absolute; - top: var(--ofe-api-header-padding); - right: var(--ofe-api-header-padding); - color: var(--ofe-api-source-link-color); - &:hover { - color: var(--pst-color-link-hover); - } - } - // Permalink to the object (to here) - > a.headerlink { - font-size: 1em; - opacity: 1; - transform: translate(0); - } - } - // Content - >dd { - margin: 1em; - &:empty { - padding-bottom: 0 !important; - } - // Description/docstring - >p { - - } - // JSON schema for pydantic stuff - >.autodoc_pydantic_collapsable_json { - } - // Parameters, Returns, Other Parameters, Raises sections - >.field-list { - // Headings - >dt { - @extend .rubric; - } - // Content - >dd { - >ul.simple { - margin-left: 0; - } - >ul.simple:first-child>li { - list-style: none; - margin-left: 0; - } - >dl>dt, >ul.simple:first-child>li>p:first-child, >p:first-child { - // Name of the parameter or return value - >strong { - font-family: var(--pst-font-family-monospace); - color: var(--ofe-api-param-name-color); - } - // Type of the parameter, or type of a named return value - >.classifier { - font-family: var(--pst-font-family-monospace); - color: var(--ofe-api-type-color); - overflow-wrap: break-word; - } - >a.reference>em { - font-family: var(--pst-font-family-monospace); - } - } - >dl>dt>strong::after { - content: ": "; - } - // Description of the parameter, return value, or exception - >dl>dd, >ul.simple:first-child>li>p:not(:first-child) { - margin-top: 0; - margin-bottom: 0.5em; - } - } - } - // Notes, References, Methods, Attributes, and Examples headings - >.rubric { - - } - // Reference list (bibliography) - >dl.citation { - - } - // Doctests - already nicely formatted! - >.doctest { - - } - // Tables of methods, attributes, classes, etc. - // Should probably style this globally so it captures the same tables in autosummary directives - >.longtable.docutils { - // A row of the table - tr { - //An odd row - *.row-odd { - - } - //An even row - *.row-even { - - } - // An entry on the LHS of the table - link to another object + possibly a signature - // Signature is a direct child of this element - td:first-child { - // May need to style everything in td:first-child, then re-overwrite things here - a.reference code { - - } - } - // An entry on the RHS of the table - description - td:last-child { - - } - - } - - } - // Child object - recapitulates structure above - >dl:not(.docutils):not(.field-list):not(.simple):not(.citation):not(.option-list):not(.footnote)[class] { - padding-bottom: 0; - box-shadow: 0 4px 5px 0 rgba(black, .14), - 0 1px 10px 0 rgba(black, .12), - 0 2px 4px -1px rgba(black, .40); - border-radius: .25rem; - border: none; - // Inner object body - > dd { - margin-right: 1.5em; - margin-left: 1.5em; - padding-bottom: 0.75em; - } - } - } - - // Don't justify/hyphenate in API - // Undoes styling found by searching "@if hyphenate" in sphinx-api.scss - p { - hyphens: none; - text-align: unset; - } -} - -// Pydantic fields have their own stuff going on -// We still want types to be the right colour -// This works as long as the type annotation is defined, -// but can cause the field's name to be recoloured if eg no -// type is defined and the field has an alias -.pydantic_field .sig { - > .sig-name + .property:not(:last-of-type) { - a { - font-weight: bold; - } - .pre { - color: var(--ofe-api-type-color); - } - } -} - -details.autodoc_pydantic_collapsable_json > summary { - max-width: 42.5rem; - margin-left: auto; - margin-right: auto; -} - -// Break headings wherever, if necessary -h1, h2, h3, h4, h5, h6 { - overflow-wrap: break-word; -} - -// Docs button in source on right -.viewcode-back { - float: right; - color: var(--ofe-api-source-link-color); -} diff --git a/docs/_static/css/custom.css b/docs/_static/css/custom.css deleted file mode 100644 index 8d6e2cf0b..000000000 --- a/docs/_static/css/custom.css +++ /dev/null @@ -1,249 +0,0 @@ -html{ - --ofe-color-BadassBlue: #31394d; - --ofe-color-OtherBlue: #002f4a; - --ofe-color-SandySergio: #d9c4b1; - --ofe-color-SergiosCousin: #ede3da; - --ofe-color-FeelingSpicy: #b85741; - --ofe-color-FeelingSick: #009384; - --ofe-color-BeastlyGrey: #666666; - - --ofe-color-GoldenYellow: #EEC044FF; - --ofe-color-DarkGoldenYellow: #c9a239; - - --ofe-color-SergiosBrigtherCousin: #fffaf5; - --ofe-color-BeastlyLightGrey: #464545; -} - -/* NavBar */ -.bd-page-width { - max-width: None; -} -.bd-header .bd-header__inner{ - background: var(--ofe-color-BadassBlue); -} - -/*NavBar links*/ -.bd-header .navbar-nav li a.nav-link{ - color: var(--pst-color-primary-text); -} -.bd-header .navbar-nav li a.nav-link:hover{ - color: var(--pst-color-primary-highlight); -} -.bd-header .navbar-nav>.active>.nav-link { - color: var(--pst-color-success); -} - -/*NavBar toggles/otherbuttons*/ -.bd-header .navbar-item{ - color: var(--pst-color-primary-text); -} -.bd-header .navbar-item:hover{ - color: var(--pst-color-primary-highlight); -} -.bd-header .theme-switch-button span{ - color: var(--pst-color-primary-text); -} -.bd-header label.sidebar-toggle{ - color: var(--pst-color-primary-text); -} -.bd-header label.sidebar-toggle:hover{ - color:var(--pst-color-primary-highlight); -} -.bd-header .search-button i{ - color: var(--pst-color-primary-text); -} -.bd-header .search-button i:hover{ - color: var(--pst-color-primary-highlight); -} - -.navbar{ - --bs-navbar-brand-hover-color: var(--ofe-color-SandySergio); -} - -.navbar img{ - border: 0px; -} -.navbar p { - color: var(--ofe-color-SergiosCousin); - -} - -img { - border: 2px solid var(--ofe-color-BeastlyGrey); - background: var(--ofe-color-SandySergio); -} - -/* -Themes - Deviating Colors are ofe-colorVars rest is original scipyData theme. -*/ - -html[data-theme=light]{ - --pst-color-primary: var(--ofe-color-OtherBlue); - --pst-color-primary-text: var(--ofe-color-SandySergio); - --pst-color-primary-highlight: var(--ofe-color-SergiosCousin); - --sd-color-primary: var(--pst-color-primary); - --sd-color-primary-text: var(--pst-color-primary-text); - --sd-color-primary-highlight: var(--pst-color-primary-highlight); - --pst-color-secondary: var(--ofe-color-SandySergio); - --pst-color-secondary-text: #fff; - --pst-color-secondary-highlight: #cf6912; - --sd-color-secondary: var(--pst-color-secondary); - --sd-color-secondary-text: var(--pst-color-secondary-text); - --sd-color-secondary-highlight: var(--pst-color-secondary-highlight); - --pst-color-success: #28a745; - --pst-color-success-text: #fff; - --pst-color-success-highlight: #19692c; - --sd-color-success: var(--pst-color-success); - --sd-color-success-text: var(--pst-color-success-text); - --sd-color-success-highlight: var(--pst-color-success-highlight); - --pst-color-info: #459db9; - --pst-color-info-text: #fff; - --pst-color-info-highlight: #306e81; - --sd-color-info: var(--pst-color-info); - --sd-color-info-text: var(--pst-color-info-text); - --sd-color-info-highlight: var(--pst-color-info-highlight); - --pst-color-warning: #ee9040; - --pst-color-warning-text: #fff; - --pst-color-warning-highlight: #cf6912; - --sd-color-warning: var(--pst-color-warning); - --sd-color-warning-text: var(--pst-color-warning-text); - --sd-color-warning-highlight: var(--pst-color-warning-highlight); - --pst-color-danger: #dc3545; - --pst-color-danger-text: #fff; - --pst-color-danger-highlight: #a71d2a; - --sd-color-danger: var(--pst-color-danger); - --sd-color-danger-text: var(--pst-color-danger-text); - --sd-color-danger-highlight: var(--pst-color-danger-highlight); - --pst-color-light: #c9c9c9; - --pst-color-light-text: #000; - --pst-color-light-highlight: #a3a3a3; - --sd-color-light: var(--pst-color-light); - --sd-color-light-text: var(--pst-color-light-text); - --sd-color-light-highlight: var(--pst-color-light-highlight); - --pst-color-muted: #646464; - --pst-color-muted-text: #fff; - --pst-color-muted-highlight: #3e3e3e; - --sd-color-muted: var(--pst-color-muted); - --sd-color-muted-text: var(--pst-color-muted-text); - --sd-color-muted-highlight: var(--pst-color-muted-highlight); - --pst-color-dark: #323232; - --pst-color-dark-text: #fff; - --pst-color-dark-highlight: #0c0c0c; - --sd-color-dark: var(--pst-color-dark); - --sd-color-dark-text: var(--pst-color-dark-text); - --sd-color-dark-highlight: var(--pst-color-dark-highlight); - --pst-color-black: #000; - --pst-color-black-text: #fff; - --pst-color-black-highlight: #000; - --sd-color-black: var(--pst-color-black); - --sd-color-black-text: var(--pst-color-black-text); - --sd-color-black-highlight: var(--pst-color-black-highlight); - --pst-color-white: #fff; - --pst-color-white-text: #000; - --pst-color-white-highlight: #d9d9d9; - --sd-color-white: var(--pst-color-white); - --sd-color-white-text: var(--pst-color-white-text); - --sd-color-white-highlight: var(--pst-color-white-highlight) -} -html[data-theme=light] { - --pst-color-attention: var(--ofe-color-GoldenYellow); - --pst-color-text-base: var(--ofe-color-OtherBlue); - --pst-color-text-muted: var(--ofe-color-BadassBlue); - --pst-color-shadow: #d8d8d8; - --pst-color-border: #c9c9c9; - --pst-color-inline-code: var(--ofe-color-FeelingSick); - --pst-color-target: var(--ofe-color-GoldenYellow); - --pst-color-background: var(--ofe-color-SergiosBrigtherCousin); - --pst-color-on-background: #fff; - --pst-color-surface: #f5f5f5; - --pst-color-on-surface: #e1e1e1; - --pst-color-link: var( --ofe-color-DarkGoldenYellow); - --pst-color-link-hover: var(--pst-color-warning) -} - -/*DarkTheme*/ -html[data-theme=dark] { - --pst-color-primary: var(--ofe-color-SergiosCousin); - --pst-color-primary-text: var(--ofe-color-SandySergio); - --pst-color-primary-highlight: #306e81; - --sd-color-primary: var(--pst-color-primary); - --sd-color-primary-text: var(--pst-color-primary-text); - --sd-color-primary-highlight: var(--pst-color-primary-highlight); - --pst-color-secondary: #ee9040; - --pst-color-secondary-text: #fff; - --pst-color-secondary-highlight: #cf6912; - --sd-color-secondary: var(--pst-color-secondary); - --sd-color-secondary-text: var(--pst-color-secondary-text); - --sd-color-secondary-highlight: var(--pst-color-secondary-highlight); - --pst-color-success: #488757; - --pst-color-success-text: #fff; - --pst-color-success-highlight: #2d5537; - --sd-color-success: var(--pst-color-success); - --sd-color-success-text: var(--pst-color-success-text); - --sd-color-success-highlight: var(--pst-color-success-highlight); - --pst-color-info: #459db9; - --pst-color-info-text: #fff; - --pst-color-info-highlight: #306e81; - --sd-color-info: var(--pst-color-info); - --sd-color-info-text: var(--pst-color-info-text); - --sd-color-info-highlight: var(--pst-color-info-highlight); - --pst-color-warning: #ee9040; - --pst-color-warning-text: #fff; - --pst-color-warning-highlight: #cf6912; - --sd-color-warning: var(--pst-color-warning); - --sd-color-warning-text: var(--pst-color-warning-text); - --sd-color-warning-highlight: var(--pst-color-warning-highlight); - --pst-color-danger: #cb4653; - --pst-color-danger-text: #fff; - --pst-color-danger-highlight: #992b36; - --sd-color-danger: var(--pst-color-danger); - --sd-color-danger-text: var(--pst-color-danger-text); - --sd-color-danger-highlight: var(--pst-color-danger-highlight); - --pst-color-light: #c9c9c9; - --pst-color-light-text: #000; - --pst-color-light-highlight: #a3a3a3; - --sd-color-light: var(--pst-color-light); - --sd-color-light-text: var(--pst-color-light-text); - --sd-color-light-highlight: var(--pst-color-light-highlight); - --pst-color-muted: #a6a6a6; - --pst-color-muted-text: #fff; - --pst-color-muted-highlight: gray; - --sd-color-muted: var(--pst-color-muted); - --sd-color-muted-text: var(--pst-color-muted-text); - --sd-color-muted-highlight: var(--pst-color-muted-highlight); - --pst-color-dark: #cecece; - --pst-color-dark-text: #000; - --pst-color-dark-highlight: #a8a8a8; - --sd-color-dark: var(--pst-color-dark); - --sd-color-dark-text: var(--pst-color-dark-text); - --sd-color-dark-highlight: var(--pst-color-dark-highlight); - --pst-color-black: #000; - --pst-color-black-text: #fff; - --pst-color-black-highlight: #000; - --sd-color-black: var(--pst-color-black); - --sd-color-black-text: var(--pst-color-black-text); - --sd-color-black-highlight: var(--pst-color-black-highlight); - --pst-color-white: #fff; - --pst-color-white-text: #000; - --pst-color-white-highlight: #d9d9d9; - --sd-color-white: var(--pst-color-white); - --sd-color-white-text: var(--pst-color-white-text); - --sd-color-white-highlight: var(--pst-color-white-highlight) -} - - -html[data-theme=dark] { - --pst-color-attention: var(--ofe-color-FeelingSpicy); - --pst-color-text-base: var(--ofe-color-SergiosBrigtherCousin); - --pst-color-text-muted: var(--ofe-color-SandySergio); - --pst-color-shadow: #212121; - --pst-color-border: silver; - --pst-color-inline-code: var(--ofe-color-SergiosCousin); - --pst-color-target: var(--ofe-color-DarkGoldenYellow); - --pst-color-background: var(--ofe-color-BeastlyLightGrey); - --pst-color-on-background: #1e1e1e; - --pst-color-surface: #666666; - --pst-color-on-surface: #373737; - --pst-color-link: var(--ofe-color-DarkGoldenYellow); - --pst-color-link-hover: var(--pst-color-warning) -} diff --git a/docs/conf.py b/docs/conf.py index dabaac218..02d0ed5b7 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -99,7 +99,7 @@ # The theme to use for HTML and HTML Help pages. See the documentation for # a list of builtin themes. # -html_theme = "pydata_sphinx_theme" +html_theme = "ofe_sphinx_theme" html_theme_options = { "logo": {"text": "OpenFE Documentation"}, "icon_links": [ diff --git a/docs/environment.yaml b/docs/environment.yaml index 7fcefaf27..5dba610b1 100644 --- a/docs/environment.yaml +++ b/docs/environment.yaml @@ -10,8 +10,7 @@ dependencies: - openmm - packaging - plugcli -- pydata-sphinx-theme -- python=3.9 +- python=3.10 - sphinx<7 - sphinx-click - tqdm @@ -21,6 +20,7 @@ dependencies: - sphinx-toolbox - sphinx<7 - git+https://github.com/OpenFreeEnergy/gufe@main + - git+https://github.com/yoshanuikabundi/ofe_sphinx_theme@main # These are added automatically by RTD, so we include them here # for a consistent environment. From 67479e2b49b7a888de3b1ceb95d745a76c630ef8 Mon Sep 17 00:00:00 2001 From: Josh Mitchell Date: Thu, 28 Sep 2023 13:04:21 +1000 Subject: [PATCH 02/28] Set accent color --- docs/conf.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/conf.py b/docs/conf.py index 02d0ed5b7..33067355f 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -104,12 +104,13 @@ "logo": {"text": "OpenFE Documentation"}, "icon_links": [ { - "name": "Github", + "name": "GitHub", "url": "https://github.com/OpenFreeEnergy/openfe", "icon": "fa-brands fa-square-github", "type": "fontawesome", } ], + "accent_color": "DarkGoldenYellow", } html_logo = "_static/Squaredcircle.svg" From 5ee4f62f1d6daac68417f139297bb4c572a7395b Mon Sep 17 00:00:00 2001 From: Josh Mitchell Date: Thu, 28 Sep 2023 13:28:41 +1000 Subject: [PATCH 03/28] Use OpenFreeEnergy namespace --- docs/environment.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/environment.yaml b/docs/environment.yaml index 5dba610b1..4f4bfae8d 100644 --- a/docs/environment.yaml +++ b/docs/environment.yaml @@ -20,7 +20,7 @@ dependencies: - sphinx-toolbox - sphinx<7 - git+https://github.com/OpenFreeEnergy/gufe@main - - git+https://github.com/yoshanuikabundi/ofe_sphinx_theme@main + - git+https://github.com/OpenFreeEnergy/ofe-sphinx-theme@main # These are added automatically by RTD, so we include them here # for a consistent environment. From f243d1452f907e7d84a0cc18364a49e9528d1fda Mon Sep 17 00:00:00 2001 From: richard gowers Date: Tue, 10 Oct 2023 08:53:08 +0100 Subject: [PATCH 04/28] matplotlib typing, use correct import for Axes --- openfe/analysis/plotting.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/openfe/analysis/plotting.py b/openfe/analysis/plotting.py index eba7b7e7f..282ef5439 100644 --- a/openfe/analysis/plotting.py +++ b/openfe/analysis/plotting.py @@ -1,12 +1,13 @@ # This code is part of OpenFE and is licensed under the MIT license. # For details, see https://github.com/OpenFreeEnergy/openfe import matplotlib.pyplot as plt +from matplotlib.axes import Axes import numpy.typing as npt from openff.units import unit from typing import Optional, Union -def plot_lambda_transition_matrix(matrix: npt.NDArray) -> plt.Axes: +def plot_lambda_transition_matrix(matrix: npt.NDArray) -> Axes: """ Plot out a transition matrix. @@ -17,7 +18,7 @@ def plot_lambda_transition_matrix(matrix: npt.NDArray) -> plt.Axes: Returns ------- - ax : matplotlib.pyplot.Axes + ax : matplotlib.axes.Axes An Axes object to plot. """ num_states = len(matrix) @@ -79,7 +80,7 @@ def plot_lambda_transition_matrix(matrix: npt.NDArray) -> plt.Axes: def plot_convergence( forward_and_reverse: dict[str, Union[npt.NDArray, unit.Quantity]], units: unit.Quantity -) -> plt.Axes: +) -> Axes: """ Plot a Reverse and Forward convergence analysis of the free energies. @@ -95,7 +96,7 @@ def plot_convergence( Returns ------- - ax : matplotlib.pyplot.Axes + ax : matplotlib.axes.Axes An Axes object to plot. """ known_units = { @@ -165,7 +166,7 @@ def plot_convergence( def plot_replica_timeseries( state_timeseries: npt.NDArray, equilibration_iterations: Optional[int] = None, -) -> plt.Axes: +) -> Axes: """ Plot a the state timeseries of a set of replicas. @@ -178,7 +179,7 @@ def plot_replica_timeseries( Returns ------- - ax : matplotlib.pyplot.Axes + ax : matplotlib.axes.Axes An Axes object to plot. """ num_states = len(state_timeseries.T) From 3f554e2e4bcc052499166340091a6694021c3bcb Mon Sep 17 00:00:00 2001 From: richard gowers Date: Tue, 10 Oct 2023 08:58:20 +0100 Subject: [PATCH 05/28] matplotlib typing, silence some warnings --- openfe/protocols/openmm_utils/multistate_analysis.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openfe/protocols/openmm_utils/multistate_analysis.py b/openfe/protocols/openmm_utils/multistate_analysis.py index 4c22f4f13..6d523f7f4 100644 --- a/openfe/protocols/openmm_utils/multistate_analysis.py +++ b/openfe/protocols/openmm_utils/multistate_analysis.py @@ -74,7 +74,7 @@ def plot(self, filepath: Path, filename_prefix: str): # MBAR overlap matrix ax = plotting.plot_lambda_transition_matrix(self.free_energy_overlaps['matrix']) ax.set_title('MBAR overlap matrix') - ax.figure.savefig( + ax.figure.savefig( # type: ignore filepath / (filename_prefix + 'mbar_overlap_matrix.png') ) @@ -83,7 +83,7 @@ def plot(self, filepath: Path, filename_prefix: str): self.forward_and_reverse_free_energies, self.units ) ax.set_title('Forward and Reverse free energy convergence') - ax.figure.savefig( + ax.figure.savefig( # type: ignore filepath / (filename_prefix + 'forward_reverse_convergence.png') ) @@ -92,7 +92,7 @@ def plot(self, filepath: Path, filename_prefix: str): self.replica_states, self.equilibration_iterations ) ax.set_title('Change in replica state over time') - ax.figure.savefig( + ax.figure.savefig( # type: ignore filepath / (filename_prefix + 'replica_state_timeseries.png') ) @@ -102,7 +102,7 @@ def plot(self, filepath: Path, filename_prefix: str): self.replica_exchange_statistics['matrix'] ) ax.set_title('Replica exchange transition matrix') - ax.figure.savefig( + ax.figure.savefig( # type: ignore filepath / (filename_prefix + 'replica_exchange_matrix.png') ) From d3d4ce5fd618c9bc536a2cc51ce5b241ebf0c0ee Mon Sep 17 00:00:00 2001 From: richard gowers Date: Tue, 10 Oct 2023 09:01:24 +0100 Subject: [PATCH 06/28] use builtin list/dict/tuple for typing --- openfe/utils/network_plotting.py | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/openfe/utils/network_plotting.py b/openfe/utils/network_plotting.py index 09bc657b1..286c2326b 100644 --- a/openfe/utils/network_plotting.py +++ b/openfe/utils/network_plotting.py @@ -16,12 +16,12 @@ from matplotlib.patches import Rectangle from matplotlib.lines import Line2D -from typing import Dict, List, Tuple, Optional, Any, Union, cast +from typing import Optional, Any, Union, cast from openfe.utils.custom_typing import ( MPL_MouseEvent, MPL_FigureCanvasBase, MPL_Axes, TypeAlias ) -ClickLocation: TypeAlias = Tuple[Tuple[float, float], Tuple[Any, Any]] +ClickLocation: TypeAlias = tuple[tuple[float, float], tuple[Any, Any]] class Node: @@ -54,14 +54,14 @@ def register_artist(self, ax: MPL_Axes): ax.add_patch(self.artist) @property - def extent(self) -> Tuple[float, float, float, float]: + def extent(self) -> tuple[float, float, float, float]: """extent of this node in matplotlib data coordinates""" bounds = self.artist.get_bbox().bounds return (bounds[0], bounds[0] + bounds[2], bounds[1], bounds[1] + bounds[3]) @property - def xy(self) -> Tuple[float, float]: + def xy(self) -> tuple[float, float]: """lower left (matplotlib data coordinates) position of this node""" return self.artist.xy @@ -153,14 +153,14 @@ class Edge: """ pickable = True - def __init__(self, node_artist1: Node, node_artist2: Node, data: Dict): + def __init__(self, node_artist1: Node, node_artist2: Node, data: dict): self.data = data self.node_artists = [node_artist1, node_artist2] self.artist = self._make_artist(node_artist1, node_artist2, data) self.picked = False def _make_artist(self, node_artist1: Node, node_artist2: Node, - data: Dict) -> Any: + data: dict) -> Any: xs, ys = self._edge_xs_ys(node_artist1, node_artist2) return Line2D(xs, ys, color='black', picker=True, zorder=-1) @@ -247,8 +247,8 @@ def __init__(self, graph: GraphDrawing): self.graph = graph self.active: Optional[Union[Node, Edge]] = None self.selected: Optional[Union[Node, Edge]] = None - self.click_location: Optional[Tuple[int, int]] = None - self.connections: List[int] = [] + self.click_location: Optional[tuple[int, int]] = None + self.connections: list[int] = [] def connect(self, canvas: MPL_FigureCanvasBase): """Connect our methods to events in the matplotlib canvas""" @@ -346,8 +346,8 @@ def __init__(self, graph: nx.Graph, positions=None, ax=None): # TODO: use scale to scale up the positions? self.event_handler = EventHandler(self) self.graph = graph - self.nodes: Dict[Node, Any] = {} - self.edges: Dict[Tuple[Node, Node], Any] = {} + self.nodes: dict[Node, Any] = {} + self.edges: dict[tuple[Node, Node], Any] = {} if positions is None: positions = nx.nx_agraph.graphviz_layout(self.graph, prog='neato') @@ -378,7 +378,7 @@ def __init__(self, graph: nx.Graph, positions=None, ax=None): def _ipython_display_(self): # -no-cov- return self.fig - def edges_for_node(self, node: Node) -> List[Edge]: + def edges_for_node(self, node: Node) -> list[Edge]: """List of edges for the given node""" edges = (list(self.graph.in_edges(node)) + list(self.graph.out_edges(node))) @@ -410,7 +410,7 @@ def draw(self): self.fig.canvas.draw() self.fig.canvas.flush_events() - def _register_node(self, node: Any, position: Tuple[float, float]): + def _register_node(self, node: Any, position: tuple[float, float]): """Create and register ``Node`` from NetworkX node and position""" if node in self.nodes: raise RuntimeError("node provided multiple times") @@ -419,7 +419,7 @@ def _register_node(self, node: Any, position: Tuple[float, float]): self.nodes[node] = draw_node draw_node.register_artist(self.ax) - def _register_edge(self, edge: Tuple[Node, Node, Dict]): + def _register_edge(self, edge: tuple[Node, Node, dict]): """Create and register ``Edge`` from NetworkX edge information""" node1, node2, data = edge draw_edge = self.EdgeCls(self.nodes[node1], self.nodes[node2], data) From 43249fd9a914c8216d410c5ea29247bc2c67fe95 Mon Sep 17 00:00:00 2001 From: richard gowers Date: Tue, 10 Oct 2023 08:59:05 +0100 Subject: [PATCH 07/28] matplotlib typing, silence some warnings change type of click_location --- openfe/utils/network_plotting.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/openfe/utils/network_plotting.py b/openfe/utils/network_plotting.py index 286c2326b..025b00cbe 100644 --- a/openfe/utils/network_plotting.py +++ b/openfe/utils/network_plotting.py @@ -238,7 +238,7 @@ class EventHandler: selected : Optional[Union[Node, Edge]] Object selected by a mouse click (after mouse is up), or None if no object has been selected in the graph. - click_location : Optional[Tuple[int, int]] + click_location : tuple[Optional[float], Optional[float]] Cached location of the mousedown event, or None if mouse is up connections : List[int] list of IDs for connections to matplotlib canvas @@ -247,15 +247,15 @@ def __init__(self, graph: GraphDrawing): self.graph = graph self.active: Optional[Union[Node, Edge]] = None self.selected: Optional[Union[Node, Edge]] = None - self.click_location: Optional[tuple[int, int]] = None + self.click_location: tuple[Optional[float], Optional[float]] = None, None self.connections: list[int] = [] def connect(self, canvas: MPL_FigureCanvasBase): """Connect our methods to events in the matplotlib canvas""" self.connections.extend([ - canvas.mpl_connect('button_press_event', self.on_mousedown), - canvas.mpl_connect('motion_notify_event', self.on_drag), - canvas.mpl_connect('button_release_event', self.on_mouseup), + canvas.mpl_connect('button_press_event', self.on_mousedown), # type: ignore + canvas.mpl_connect('motion_notify_event', self.on_drag), # type: ignore + canvas.mpl_connect('button_release_event', self.on_mouseup), # type: ignore ]) def disconnect(self, canvas: MPL_FigureCanvasBase): @@ -319,7 +319,7 @@ def on_mouseup(self, event: MPL_MouseEvent): self.active.on_mouseup(event, self.graph) self.active = None - self.click_location = None + self.click_location = None, None self.graph.draw() From 6ce00ddd9cc6dcc1d67f599610577e1563f2128a Mon Sep 17 00:00:00 2001 From: richard gowers Date: Tue, 10 Oct 2023 09:13:03 +0100 Subject: [PATCH 08/28] fixup typing in network_plotting --- openfe/utils/network_plotting.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openfe/utils/network_plotting.py b/openfe/utils/network_plotting.py index 025b00cbe..ad767dc73 100644 --- a/openfe/utils/network_plotting.py +++ b/openfe/utils/network_plotting.py @@ -238,7 +238,7 @@ class EventHandler: selected : Optional[Union[Node, Edge]] Object selected by a mouse click (after mouse is up), or None if no object has been selected in the graph. - click_location : tuple[Optional[float], Optional[float]] + click_location : Optional[tuple[Optional[float], Optional[float]]] Cached location of the mousedown event, or None if mouse is up connections : List[int] list of IDs for connections to matplotlib canvas @@ -247,7 +247,7 @@ def __init__(self, graph: GraphDrawing): self.graph = graph self.active: Optional[Union[Node, Edge]] = None self.selected: Optional[Union[Node, Edge]] = None - self.click_location: tuple[Optional[float], Optional[float]] = None, None + self.click_location: Optional[tuple[Optional[float], Optional[float]]] = None self.connections: list[int] = [] def connect(self, canvas: MPL_FigureCanvasBase): @@ -319,7 +319,7 @@ def on_mouseup(self, event: MPL_MouseEvent): self.active.on_mouseup(event, self.graph) self.active = None - self.click_location = None, None + self.click_location = None self.graph.draw() From f2be68a871a6ded19f6e11dbaab782f0337d6d1a Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Fri, 15 Sep 2023 15:29:51 +0200 Subject: [PATCH 09/28] add empty function with adjusted docstring --- openfe/setup/ligand_network_planning.py | 49 +++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/openfe/setup/ligand_network_planning.py b/openfe/setup/ligand_network_planning.py index 1c6265868..7d5c524dd 100644 --- a/openfe/setup/ligand_network_planning.py +++ b/openfe/setup/ligand_network_planning.py @@ -228,6 +228,55 @@ def generate_minimal_spanning_network( return min_network +def generate_minimal_redundant_network( + ligands: Iterable[SmallMoleculeComponent], + mappers: Union[AtomMapper, Iterable[AtomMapper]], + scorer: Callable[[LigandAtomMapping], float], + progress: Union[bool, Callable[[Iterable], Iterable]] = True, +) -> LigandNetwork: + """ + Plan a network with as few edges as possible with maximum total score + + Parameters + ---------- + ligands : Iterable[SmallMoleculeComponent] + the ligands to include in the LigandNetwork + mappers : AtomMapper or Iterable[AtomMapper] + the AtomMapper(s) to use to propose mappings. At least 1 required, + but many can be given, in which case all will be tried to find the + highest score edges + scorer : Scoring function + any callable which takes a LigandAtomMapping and returns a float + progress : Union[bool, Callable[Iterable], Iterable] + progress bar: if False, no progress bar will be shown. If True, use a + tqdm progress bar that only appears after 1.5 seconds. You can also + provide a custom progress bar wrapper as a callable. + """ + if isinstance(mappers, AtomMapper): + mappers = [mappers] + mappers = [_hasten_lomap(m, ligands) if isinstance(m, LomapAtomMapper) + else m for m in mappers] + + # First create a network with all the proposed mappings (scored) + network = generate_maximal_network(ligands, mappers, scorer, progress) + + # Flip network scores so we can use minimal algorithm + g2 = nx.MultiGraph() + for e1, e2, d in network.graph.edges(data=True): + g2.add_edge(e1, e2, weight=-d['score'], object=d['object']) + + # Next analyze that network to create minimal spanning network. Because + # we carry the original (directed) LigandAtomMapping, we don't lose + # direction information when converting to an undirected graph. + min_edges = nx.minimum_spanning_edges(g2) + min_mappings = [edge_data['object'] for _, _, _, edge_data in min_edges] + min_network = LigandNetwork(min_mappings) + missing_nodes = set(network.nodes) - set(min_network.nodes) + if missing_nodes: + raise RuntimeError("Unable to create edges to some nodes: " + f"{list(missing_nodes)}") + + return min_network def generate_network_from_names( ligands: list[SmallMoleculeComponent], From f6a4bdf4b603a8e2d0d9c90df9ec02e5097702a5 Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Fri, 15 Sep 2023 15:31:22 +0200 Subject: [PATCH 10/28] remove bulk of function, will add back gradually. --- openfe/setup/ligand_network_planning.py | 27 +++---------------------- 1 file changed, 3 insertions(+), 24 deletions(-) diff --git a/openfe/setup/ligand_network_planning.py b/openfe/setup/ligand_network_planning.py index 7d5c524dd..75be0167d 100644 --- a/openfe/setup/ligand_network_planning.py +++ b/openfe/setup/ligand_network_planning.py @@ -235,7 +235,9 @@ def generate_minimal_redundant_network( progress: Union[bool, Callable[[Iterable], Iterable]] = True, ) -> LigandNetwork: """ - Plan a network with as few edges as possible with maximum total score + Plan a network with as few edges as possible with maximum total score, + ensuring that every node is connected to two edges to introduce + statistical redundancy. Parameters ---------- @@ -252,29 +254,6 @@ def generate_minimal_redundant_network( tqdm progress bar that only appears after 1.5 seconds. You can also provide a custom progress bar wrapper as a callable. """ - if isinstance(mappers, AtomMapper): - mappers = [mappers] - mappers = [_hasten_lomap(m, ligands) if isinstance(m, LomapAtomMapper) - else m for m in mappers] - - # First create a network with all the proposed mappings (scored) - network = generate_maximal_network(ligands, mappers, scorer, progress) - - # Flip network scores so we can use minimal algorithm - g2 = nx.MultiGraph() - for e1, e2, d in network.graph.edges(data=True): - g2.add_edge(e1, e2, weight=-d['score'], object=d['object']) - - # Next analyze that network to create minimal spanning network. Because - # we carry the original (directed) LigandAtomMapping, we don't lose - # direction information when converting to an undirected graph. - min_edges = nx.minimum_spanning_edges(g2) - min_mappings = [edge_data['object'] for _, _, _, edge_data in min_edges] - min_network = LigandNetwork(min_mappings) - missing_nodes = set(network.nodes) - set(min_network.nodes) - if missing_nodes: - raise RuntimeError("Unable to create edges to some nodes: " - f"{list(missing_nodes)}") return min_network From 209f2d30c4c983914244a4317c41b9ac5fc67fc8 Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Mon, 18 Sep 2023 11:25:02 +0200 Subject: [PATCH 11/28] drop in redundant network fn --- openfe/setup/ligand_network_planning.py | 32 ++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/openfe/setup/ligand_network_planning.py b/openfe/setup/ligand_network_planning.py index 75be0167d..8766124bf 100644 --- a/openfe/setup/ligand_network_planning.py +++ b/openfe/setup/ligand_network_planning.py @@ -254,8 +254,38 @@ def generate_minimal_redundant_network( tqdm progress bar that only appears after 1.5 seconds. You can also provide a custom progress bar wrapper as a callable. """ + if isinstance(mappers, AtomMapper): + mappers = [mappers] + mappers = [_hasten_lomap(m, ligands) if isinstance(m, LomapAtomMapper) + else m for m in mappers] - return min_network + # First create a network with all the proposed mappings (scored) + network = generate_maximal_network(ligands, mappers, scorer, progress) + + # Flip network scores so we can use minimal algorithm + g2 = nx.MultiGraph() + for e1, e2, d in network.graph.edges(data=True): + g2.add_edge(e1, e2, weight=-d['score'], object=d['object']) + + # As in .generate_minimal_spanning_network(), use nx to get the minimal + # network. But now also remove those edges from the fully-connected + # network, then get the minimal network again. Add mappings from all + # minimal networks together. + mappings = [] + for _ in range(2): # can increase range here for more redundancy + # get list from generator so that we don't adjust network by calling it: + current_best_edges = list(nx.minimum_spanning_edges(g2)) + + g2.remove_edges_from(current_best_edges) + [mappings.append(edge_data['object']) for _, _, _, edge_data in current_best_edges] + + redund_network = LigandNetwork(mappings) + missing_nodes = set(network.nodes) - set(redund_network.nodes) + if missing_nodes: + raise RuntimeError("Unable to create edges to some nodes: " + f"{list(missing_nodes)}") + + return redund_network def generate_network_from_names( ligands: list[SmallMoleculeComponent], From e478b64b93ae28fecbb34b276daf43056f4b9b00 Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Mon, 18 Sep 2023 15:44:41 +0200 Subject: [PATCH 12/28] add tests --- openfe/tests/setup/test_network_planning.py | 77 +++++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/openfe/tests/setup/test_network_planning.py b/openfe/tests/setup/test_network_planning.py index 7e1e38e0d..a6f9a5e0b 100644 --- a/openfe/tests/setup/test_network_planning.py +++ b/openfe/tests/setup/test_network_planning.py @@ -244,6 +244,83 @@ def scorer(mapping): scorer=scorer ) +@pytest.fixture(scope='session') +def generate_minimal_redundant_network(toluene_vs_others): + toluene, others = toluene_vs_others + mappers = [BadMapper(), openfe.setup.atom_mapping.LomapAtomMapper()] + + def scorer(mapping): + return len(mapping.componentA_to_componentB) + + network = openfe.setup.ligand_network_planning.generate_generate_minimal_redundant_network( + ligands=others + [toluene], + mappers=mappers, + scorer=scorer + ) + return network + +def test_generate_minimal_redundant_network(generate_minimal_redundant_network, toluene_vs_others): + tol, others = toluene_vs_others + assert len(generate_minimal_redundant_network.nodes) == len(others) + 1 + for edge in generate_minimal_redundant_network.edges: + assert edge.componentA_to_componentB != {0: 0} # lomap should find something + + +def test_generate_minimal_redundant_network_connectedness(minimal_redundant_network): + found_pairs = set() + for edge in generate_minimal_redundant_network.edges: + pair = frozenset([edge.componentA, edge.componentB]) + assert pair not in found_pairs + found_pairs.add(pair) + + assert nx.is_connected(nx.MultiGraph(generate_minimal_redundant_network.graph)) + + +def generate_minimal_redundant_network(generate_minimal_redundant_network): + # issue #244, this was previously giving non-reproducible (yet valid) + # networks when scores were tied. + edge_ids = sorted( + (edge.componentA.name, edge.componentB.name) + for edge in generate_minimal_redundant_network.edges + ) + ref = sorted([ + ('2-naftanol', '2-methyl-6-propylnaphthalene'), + ('2-naftanol', 'methylcyclohexane'), + ('2-methyl-6-propylnaphthalene', 'toluene'), + ('2-naftanol', 'toluene'), + ('2-naftanol', '1-butyl-4-methylbenzene'), + ('2-methyl-6-propylnaphthalene', '1-butyl-4-methylbenzene'), + ('2-naftanol', '1,3,7-trimethylnaphthalene'), + ('2,6-dimethylnaphthalene', '2-naftanol'), + ('2-methylnaphthalene', '2-methyl-6-propylnaphthalene'), + ('methylcyclohexane', '2-methyl-6-propylnaphthalene'), + ('2,6-dimethylnaphthalene', '2-methyl-6-propylnaphthalene'), + ('2-naftanol', '2-methylnaphthalene'), + ('1,3,7-trimethylnaphthalene', '2-methyl-6-propylnaphthalene'), + ]) + + assert len(edge_ids) == len(ref) + assert edge_ids == ref + +def test_minimal_redundant_network_redundant(generate_minimal_redundant_network): + # test that each node is connected to 2 edges. + network = generate_minimal_redundant_network + for node in network.nodes: + assert len(network.graph.in_edges(node)) + len(network.graph.out_edges(node)) >= 2 + +def test_minimal_redundant_network_unreachable(toluene_vs_others): + toluene, others = toluene_vs_others + nimrod = openfe.SmallMoleculeComponent(mol_from_smiles("N")) + + def scorer(mapping): + return len(mapping.componentA_to_componentB) + + with pytest.raises(RuntimeError, match="Unable to create edges"): + network = openfe.setup.ligand_network_planning.generate_generate_minimal_redundant_network( + ligands=others + [toluene, nimrod], + mappers=[openfe.setup.atom_mapping.LomapAtomMapper()], + scorer=scorer + ) def test_network_from_names(atom_mapping_basic_test_files): ligs = list(atom_mapping_basic_test_files.values()) From 09f7d7f13391771784a7f13ee60a76d34ed20eb7 Mon Sep 17 00:00:00 2001 From: JenkeScheen <43140137+JenkeScheen@users.noreply.github.com> Date: Thu, 21 Sep 2023 09:49:30 +0200 Subject: [PATCH 13/28] Ah, missed this one (thanks mypy): Suggested change [mappings.append(edge_data['object']) for _, _, _, edge_data in current_best_edges] for _, _, _, edge_data in current_best_edges: mappings.append(edge_data['object']) Don't use a list comprehension unless you're assigning it to something. Co-authored-by: David W.H. Swenson --- openfe/setup/ligand_network_planning.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/openfe/setup/ligand_network_planning.py b/openfe/setup/ligand_network_planning.py index 8766124bf..b2a019554 100644 --- a/openfe/setup/ligand_network_planning.py +++ b/openfe/setup/ligand_network_planning.py @@ -277,7 +277,8 @@ def generate_minimal_redundant_network( current_best_edges = list(nx.minimum_spanning_edges(g2)) g2.remove_edges_from(current_best_edges) - [mappings.append(edge_data['object']) for _, _, _, edge_data in current_best_edges] + for _, _, _, edge_data in current_best_edges: + mappings.append(edge_data['object']) redund_network = LigandNetwork(mappings) missing_nodes = set(network.nodes) - set(redund_network.nodes) From e931f4b6b34c2eb305d7e59a250e085e7a58aa0e Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Thu, 21 Sep 2023 09:58:35 +0200 Subject: [PATCH 14/28] more descriptive test fixture --- openfe/tests/setup/test_network_planning.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openfe/tests/setup/test_network_planning.py b/openfe/tests/setup/test_network_planning.py index a6f9a5e0b..42c956786 100644 --- a/openfe/tests/setup/test_network_planning.py +++ b/openfe/tests/setup/test_network_planning.py @@ -176,7 +176,7 @@ def scorer(mapping): assert list(network.edges) -@pytest.fixture(scope='session') +@pytest.fixture(scope='minimal_redundant_network') def minimal_spanning_network(toluene_vs_others): toluene, others = toluene_vs_others mappers = [BadMapper(), openfe.setup.atom_mapping.LomapAtomMapper()] From 665ad33ae7458e4ea64611e7ec7e650b5405beb5 Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Thu, 21 Sep 2023 09:59:46 +0200 Subject: [PATCH 15/28] set 'test_' for actual test function.. --- openfe/tests/setup/test_network_planning.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/openfe/tests/setup/test_network_planning.py b/openfe/tests/setup/test_network_planning.py index 42c956786..f07ef6a23 100644 --- a/openfe/tests/setup/test_network_planning.py +++ b/openfe/tests/setup/test_network_planning.py @@ -176,7 +176,7 @@ def scorer(mapping): assert list(network.edges) -@pytest.fixture(scope='minimal_redundant_network') +@pytest.fixture(scope='session') def minimal_spanning_network(toluene_vs_others): toluene, others = toluene_vs_others mappers = [BadMapper(), openfe.setup.atom_mapping.LomapAtomMapper()] @@ -244,7 +244,7 @@ def scorer(mapping): scorer=scorer ) -@pytest.fixture(scope='session') +@pytest.fixture(scope='minimal_redundant_network') def generate_minimal_redundant_network(toluene_vs_others): toluene, others = toluene_vs_others mappers = [BadMapper(), openfe.setup.atom_mapping.LomapAtomMapper()] @@ -276,7 +276,7 @@ def test_generate_minimal_redundant_network_connectedness(minimal_redundant_netw assert nx.is_connected(nx.MultiGraph(generate_minimal_redundant_network.graph)) -def generate_minimal_redundant_network(generate_minimal_redundant_network): +def test_generate_minimal_redundant_network(generate_minimal_redundant_network): # issue #244, this was previously giving non-reproducible (yet valid) # networks when scores were tied. edge_ids = sorted( From a02cda425c83fd5efe24b08fcb40f72db6556e74 Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Thu, 21 Sep 2023 10:03:53 +0200 Subject: [PATCH 16/28] add test for number of edges in redundant networks --- openfe/tests/setup/test_network_planning.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/openfe/tests/setup/test_network_planning.py b/openfe/tests/setup/test_network_planning.py index f07ef6a23..6bab0f344 100644 --- a/openfe/tests/setup/test_network_planning.py +++ b/openfe/tests/setup/test_network_planning.py @@ -261,7 +261,13 @@ def scorer(mapping): def test_generate_minimal_redundant_network(generate_minimal_redundant_network, toluene_vs_others): tol, others = toluene_vs_others + + # test for correct number of nodes assert len(generate_minimal_redundant_network.nodes) == len(others) + 1 + + # test for correct number of edges + assert len(generate_minimal_redundant_network.edges) == 2 * (len(generate_minimal_redundant_network.nodes) -1) + for edge in generate_minimal_redundant_network.edges: assert edge.componentA_to_componentB != {0: 0} # lomap should find something From 3b613afa43d03fddac0b2a0e0bcd1f5e38b98a8e Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Thu, 21 Sep 2023 10:11:15 +0200 Subject: [PATCH 17/28] add mst_num = 2 by default --- openfe/setup/ligand_network_planning.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/openfe/setup/ligand_network_planning.py b/openfe/setup/ligand_network_planning.py index b2a019554..18d3b5e8d 100644 --- a/openfe/setup/ligand_network_planning.py +++ b/openfe/setup/ligand_network_planning.py @@ -233,6 +233,7 @@ def generate_minimal_redundant_network( mappers: Union[AtomMapper, Iterable[AtomMapper]], scorer: Callable[[LigandAtomMapping], float], progress: Union[bool, Callable[[Iterable], Iterable]] = True, + mst_num: int = 2, ) -> LigandNetwork: """ Plan a network with as few edges as possible with maximum total score, @@ -253,6 +254,10 @@ def generate_minimal_redundant_network( progress bar: if False, no progress bar will be shown. If True, use a tqdm progress bar that only appears after 1.5 seconds. You can also provide a custom progress bar wrapper as a callable. + mst_num: int + Minimum Spanning Tree number: the number of minimum spanning trees to + generate. If two, the second-best edges are included in the returned + network. If three, the third-best edges are also included, etc. """ if isinstance(mappers, AtomMapper): mappers = [mappers] @@ -272,7 +277,7 @@ def generate_minimal_redundant_network( # network, then get the minimal network again. Add mappings from all # minimal networks together. mappings = [] - for _ in range(2): # can increase range here for more redundancy + for _ in range(mst_num): # can increase range here for more redundancy # get list from generator so that we don't adjust network by calling it: current_best_edges = list(nx.minimum_spanning_edges(g2)) From 1d55ed9e1fe8b4268b030931c7015a6277648fa7 Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Thu, 21 Sep 2023 10:17:30 +0200 Subject: [PATCH 18/28] fix function names in redundant ntwk tests --- openfe/tests/setup/test_network_planning.py | 29 ++++++++++----------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/openfe/tests/setup/test_network_planning.py b/openfe/tests/setup/test_network_planning.py index 6bab0f344..1e4250342 100644 --- a/openfe/tests/setup/test_network_planning.py +++ b/openfe/tests/setup/test_network_planning.py @@ -245,49 +245,48 @@ def scorer(mapping): ) @pytest.fixture(scope='minimal_redundant_network') -def generate_minimal_redundant_network(toluene_vs_others): +def minimal_redundant_network(toluene_vs_others): toluene, others = toluene_vs_others mappers = [BadMapper(), openfe.setup.atom_mapping.LomapAtomMapper()] def scorer(mapping): return len(mapping.componentA_to_componentB) - network = openfe.setup.ligand_network_planning.generate_generate_minimal_redundant_network( + network = openfe.setup.ligand_network_planning.generate_minimal_redundant_network( ligands=others + [toluene], mappers=mappers, scorer=scorer ) return network -def test_generate_minimal_redundant_network(generate_minimal_redundant_network, toluene_vs_others): +def test_minimal_redundant_network(minimal_redundant_network, toluene_vs_others): tol, others = toluene_vs_others # test for correct number of nodes - assert len(generate_minimal_redundant_network.nodes) == len(others) + 1 + assert len(minimal_redundant_network.nodes) == len(others) + 1 # test for correct number of edges - assert len(generate_minimal_redundant_network.edges) == 2 * (len(generate_minimal_redundant_network.nodes) -1) + assert len(minimal_redundant_network.edges) == 2 * (len(minimal_redundant_network.nodes) -1) - for edge in generate_minimal_redundant_network.edges: + for edge in minimal_redundant_network.edges: assert edge.componentA_to_componentB != {0: 0} # lomap should find something - -def test_generate_minimal_redundant_network_connectedness(minimal_redundant_network): +def test_minimal_redundant_network_connectedness(minimal_redundant_network): found_pairs = set() - for edge in generate_minimal_redundant_network.edges: + for edge in minimal_redundant_network.edges: pair = frozenset([edge.componentA, edge.componentB]) assert pair not in found_pairs found_pairs.add(pair) - assert nx.is_connected(nx.MultiGraph(generate_minimal_redundant_network.graph)) + assert nx.is_connected(nx.MultiGraph(minimal_redundant_network.graph)) -def test_generate_minimal_redundant_network(generate_minimal_redundant_network): +def test_minimal_redundant_network(minimal_redundant_network): # issue #244, this was previously giving non-reproducible (yet valid) # networks when scores were tied. edge_ids = sorted( (edge.componentA.name, edge.componentB.name) - for edge in generate_minimal_redundant_network.edges + for edge in minimal_redundant_network.edges ) ref = sorted([ ('2-naftanol', '2-methyl-6-propylnaphthalene'), @@ -308,9 +307,9 @@ def test_generate_minimal_redundant_network(generate_minimal_redundant_network): assert len(edge_ids) == len(ref) assert edge_ids == ref -def test_minimal_redundant_network_redundant(generate_minimal_redundant_network): +def test_minimal_redundant_network_redundant(minimal_redundant_network): # test that each node is connected to 2 edges. - network = generate_minimal_redundant_network + network = minimal_redundant_network for node in network.nodes: assert len(network.graph.in_edges(node)) + len(network.graph.out_edges(node)) >= 2 @@ -322,7 +321,7 @@ def scorer(mapping): return len(mapping.componentA_to_componentB) with pytest.raises(RuntimeError, match="Unable to create edges"): - network = openfe.setup.ligand_network_planning.generate_generate_minimal_redundant_network( + network = openfe.setup.ligand_network_planning.generate_minimal_redundant_network( ligands=others + [toluene, nimrod], mappers=[openfe.setup.atom_mapping.LomapAtomMapper()], scorer=scorer From 10831d2289c87cc2175f5b36ab971b02b3f37804 Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Thu, 21 Sep 2023 10:19:06 +0200 Subject: [PATCH 19/28] add `mst_num` to default test --- openfe/tests/setup/test_network_planning.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/openfe/tests/setup/test_network_planning.py b/openfe/tests/setup/test_network_planning.py index 1e4250342..72953def4 100644 --- a/openfe/tests/setup/test_network_planning.py +++ b/openfe/tests/setup/test_network_planning.py @@ -245,7 +245,7 @@ def scorer(mapping): ) @pytest.fixture(scope='minimal_redundant_network') -def minimal_redundant_network(toluene_vs_others): +def minimal_redundant_network(toluene_vs_others, mst_num=2): toluene, others = toluene_vs_others mappers = [BadMapper(), openfe.setup.atom_mapping.LomapAtomMapper()] @@ -255,7 +255,8 @@ def scorer(mapping): network = openfe.setup.ligand_network_planning.generate_minimal_redundant_network( ligands=others + [toluene], mappers=mappers, - scorer=scorer + scorer=scorer, + mst_num=mst_num ) return network @@ -280,6 +281,10 @@ def test_minimal_redundant_network_connectedness(minimal_redundant_network): assert nx.is_connected(nx.MultiGraph(minimal_redundant_network.graph)) +def test_redundant_vs_spanning_network(minimal_redundant_network, toluene_vs_others): + + # test for correct number of edges + assert len(minimal_redundant_network.edges) == 2 * (len(minimal_redundant_network.nodes) -1) def test_minimal_redundant_network(minimal_redundant_network): # issue #244, this was previously giving non-reproducible (yet valid) From 688d063c707cb6731fa407f529df974ef465af0b Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Thu, 21 Sep 2023 10:22:55 +0200 Subject: [PATCH 20/28] test that redundant(n=1) is same as spanning --- openfe/tests/setup/test_network_planning.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/openfe/tests/setup/test_network_planning.py b/openfe/tests/setup/test_network_planning.py index 72953def4..1af8cbe47 100644 --- a/openfe/tests/setup/test_network_planning.py +++ b/openfe/tests/setup/test_network_planning.py @@ -281,10 +281,11 @@ def test_minimal_redundant_network_connectedness(minimal_redundant_network): assert nx.is_connected(nx.MultiGraph(minimal_redundant_network.graph)) -def test_redundant_vs_spanning_network(minimal_redundant_network, toluene_vs_others): +def test_redundant_vs_spanning_network(minimal_redundant_network, minimal_spanning_network): + # when setting minimal redundant network to only take one MST, it should have as many + # edges as the regular minimum spanning network + assert len(minimal_spanning_network) == len(minimal_redundant_network(mst_num=1)) - # test for correct number of edges - assert len(minimal_redundant_network.edges) == 2 * (len(minimal_redundant_network.nodes) -1) def test_minimal_redundant_network(minimal_redundant_network): # issue #244, this was previously giving non-reproducible (yet valid) From 28e444ffbd95b38d9ce5dd121e2e72634e1d7580 Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Thu, 21 Sep 2023 10:24:45 +0200 Subject: [PATCH 21/28] pep8 shenanigans --- openfe/setup/ligand_network_planning.py | 12 ++++--- openfe/tests/setup/test_network_planning.py | 36 ++++++++++++++------- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/openfe/setup/ligand_network_planning.py b/openfe/setup/ligand_network_planning.py index 18d3b5e8d..385aee990 100644 --- a/openfe/setup/ligand_network_planning.py +++ b/openfe/setup/ligand_network_planning.py @@ -160,7 +160,7 @@ def generate_maximal_network( total = len(nodes) * (len(nodes) - 1) // 2 progress = functools.partial(tqdm, total=total, delay=1.5) elif progress is False: - progress = lambda x: x + def progress(x): return x # otherwise, it should be a user-defined callable mapping_generator = itertools.chain.from_iterable( @@ -228,6 +228,7 @@ def generate_minimal_spanning_network( return min_network + def generate_minimal_redundant_network( ligands: Iterable[SmallMoleculeComponent], mappers: Union[AtomMapper, Iterable[AtomMapper]], @@ -271,13 +272,13 @@ def generate_minimal_redundant_network( g2 = nx.MultiGraph() for e1, e2, d in network.graph.edges(data=True): g2.add_edge(e1, e2, weight=-d['score'], object=d['object']) - - # As in .generate_minimal_spanning_network(), use nx to get the minimal + + # As in .generate_minimal_spanning_network(), use nx to get the minimal # network. But now also remove those edges from the fully-connected # network, then get the minimal network again. Add mappings from all # minimal networks together. mappings = [] - for _ in range(mst_num): # can increase range here for more redundancy + for _ in range(mst_num): # can increase range here for more redundancy # get list from generator so that we don't adjust network by calling it: current_best_edges = list(nx.minimum_spanning_edges(g2)) @@ -293,6 +294,7 @@ def generate_minimal_redundant_network( return redund_network + def generate_network_from_names( ligands: list[SmallMoleculeComponent], mapper: AtomMapper, @@ -417,7 +419,7 @@ def load_orion_network( KeyError If an unexpected line format is encountered. """ - + with open(network_file, 'r') as f: network_lines = [l.strip().split(' ') for l in f if not l.startswith('#')] diff --git a/openfe/tests/setup/test_network_planning.py b/openfe/tests/setup/test_network_planning.py index 1af8cbe47..89c6e0491 100644 --- a/openfe/tests/setup/test_network_planning.py +++ b/openfe/tests/setup/test_network_planning.py @@ -76,7 +76,8 @@ def scorer(mapping): assert len(network.edges) == len(others) for edge in network.edges: - assert len(edge.componentA_to_componentB) > 1 # we didn't take the bad mapper + # we didn't take the bad mapper + assert len(edge.componentA_to_componentB) > 1 assert 'score' in edge.annotations assert edge.annotations['score'] == len(edge.componentA_to_componentB) @@ -196,7 +197,8 @@ def test_minimal_spanning_network(minimal_spanning_network, toluene_vs_others): tol, others = toluene_vs_others assert len(minimal_spanning_network.nodes) == len(others) + 1 for edge in minimal_spanning_network.edges: - assert edge.componentA_to_componentB != {0: 0} # lomap should find something + assert edge.componentA_to_componentB != { + 0: 0} # lomap should find something def test_minimal_spanning_network_connectedness(minimal_spanning_network): @@ -244,6 +246,7 @@ def scorer(mapping): scorer=scorer ) + @pytest.fixture(scope='minimal_redundant_network') def minimal_redundant_network(toluene_vs_others, mst_num=2): toluene, others = toluene_vs_others @@ -260,6 +263,7 @@ def scorer(mapping): ) return network + def test_minimal_redundant_network(minimal_redundant_network, toluene_vs_others): tol, others = toluene_vs_others @@ -267,10 +271,13 @@ def test_minimal_redundant_network(minimal_redundant_network, toluene_vs_others) assert len(minimal_redundant_network.nodes) == len(others) + 1 # test for correct number of edges - assert len(minimal_redundant_network.edges) == 2 * (len(minimal_redundant_network.nodes) -1) + assert len(minimal_redundant_network.edges) == 2 * \ + (len(minimal_redundant_network.nodes) - 1) for edge in minimal_redundant_network.edges: - assert edge.componentA_to_componentB != {0: 0} # lomap should find something + assert edge.componentA_to_componentB != { + 0: 0} # lomap should find something + def test_minimal_redundant_network_connectedness(minimal_redundant_network): found_pairs = set() @@ -281,11 +288,13 @@ def test_minimal_redundant_network_connectedness(minimal_redundant_network): assert nx.is_connected(nx.MultiGraph(minimal_redundant_network.graph)) + def test_redundant_vs_spanning_network(minimal_redundant_network, minimal_spanning_network): # when setting minimal redundant network to only take one MST, it should have as many # edges as the regular minimum spanning network - assert len(minimal_spanning_network) == len(minimal_redundant_network(mst_num=1)) - + assert len(minimal_spanning_network) == len( + minimal_redundant_network(mst_num=1)) + def test_minimal_redundant_network(minimal_redundant_network): # issue #244, this was previously giving non-reproducible (yet valid) @@ -313,12 +322,15 @@ def test_minimal_redundant_network(minimal_redundant_network): assert len(edge_ids) == len(ref) assert edge_ids == ref + def test_minimal_redundant_network_redundant(minimal_redundant_network): # test that each node is connected to 2 edges. network = minimal_redundant_network for node in network.nodes: - assert len(network.graph.in_edges(node)) + len(network.graph.out_edges(node)) >= 2 - + assert len(network.graph.in_edges(node)) + \ + len(network.graph.out_edges(node)) >= 2 + + def test_minimal_redundant_network_unreachable(toluene_vs_others): toluene, others = toluene_vs_others nimrod = openfe.SmallMoleculeComponent(mol_from_smiles("N")) @@ -333,6 +345,7 @@ def scorer(mapping): scorer=scorer ) + def test_network_from_names(atom_mapping_basic_test_files): ligs = list(atom_mapping_basic_test_files.values()) @@ -454,10 +467,12 @@ def test_network_from_external(file_fixture, loader, request, expected_edges = { (benzene_modifications['benzene'], benzene_modifications['toluene']), (benzene_modifications['benzene'], benzene_modifications['phenol']), - (benzene_modifications['benzene'], benzene_modifications['benzonitrile']), + (benzene_modifications['benzene'], + benzene_modifications['benzonitrile']), (benzene_modifications['benzene'], benzene_modifications['anisole']), (benzene_modifications['benzene'], benzene_modifications['styrene']), - (benzene_modifications['benzene'], benzene_modifications['benzaldehyde']), + (benzene_modifications['benzene'], + benzene_modifications['benzaldehyde']), } actual_edges = {(e.componentA, e.componentB) for e in list(network.edges)} @@ -511,7 +526,6 @@ def test_bad_orion_network(benzene_modifications, tmpdir): ) - BAD_EDGES = """\ 1c91235:9c91235 benzene -> toluene 1c91235:7876633 benzene -> phenol From b0639d43d3162b73d71ad27a80e194f9672df5b3 Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Thu, 21 Sep 2023 10:32:44 +0200 Subject: [PATCH 22/28] redundant network into original MST test scope --- openfe/tests/setup/test_network_planning.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/openfe/tests/setup/test_network_planning.py b/openfe/tests/setup/test_network_planning.py index 89c6e0491..384e37039 100644 --- a/openfe/tests/setup/test_network_planning.py +++ b/openfe/tests/setup/test_network_planning.py @@ -246,8 +246,6 @@ def scorer(mapping): scorer=scorer ) - -@pytest.fixture(scope='minimal_redundant_network') def minimal_redundant_network(toluene_vs_others, mst_num=2): toluene, others = toluene_vs_others mappers = [BadMapper(), openfe.setup.atom_mapping.LomapAtomMapper()] From 3168be606918afb56f29fba3a26b11bbfe3c9af0 Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Thu, 21 Sep 2023 10:41:19 +0200 Subject: [PATCH 23/28] revert scope back to `session` so we can pull data --- openfe/tests/setup/test_network_planning.py | 1 + 1 file changed, 1 insertion(+) diff --git a/openfe/tests/setup/test_network_planning.py b/openfe/tests/setup/test_network_planning.py index 384e37039..66e725e0b 100644 --- a/openfe/tests/setup/test_network_planning.py +++ b/openfe/tests/setup/test_network_planning.py @@ -246,6 +246,7 @@ def scorer(mapping): scorer=scorer ) +@pytest.fixture(scope='session') def minimal_redundant_network(toluene_vs_others, mst_num=2): toluene, others = toluene_vs_others mappers = [BadMapper(), openfe.setup.atom_mapping.LomapAtomMapper()] From 1a782c722f9ad286dae1a3d3f9b847d085934005 Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Thu, 21 Sep 2023 10:48:17 +0200 Subject: [PATCH 24/28] test fix (had wrong network refs) --- openfe/tests/setup/test_network_planning.py | 27 +++++++++++---------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/openfe/tests/setup/test_network_planning.py b/openfe/tests/setup/test_network_planning.py index 66e725e0b..53411c577 100644 --- a/openfe/tests/setup/test_network_planning.py +++ b/openfe/tests/setup/test_network_planning.py @@ -303,19 +303,20 @@ def test_minimal_redundant_network(minimal_redundant_network): for edge in minimal_redundant_network.edges ) ref = sorted([ - ('2-naftanol', '2-methyl-6-propylnaphthalene'), - ('2-naftanol', 'methylcyclohexane'), - ('2-methyl-6-propylnaphthalene', 'toluene'), - ('2-naftanol', 'toluene'), - ('2-naftanol', '1-butyl-4-methylbenzene'), - ('2-methyl-6-propylnaphthalene', '1-butyl-4-methylbenzene'), - ('2-naftanol', '1,3,7-trimethylnaphthalene'), - ('2,6-dimethylnaphthalene', '2-naftanol'), - ('2-methylnaphthalene', '2-methyl-6-propylnaphthalene'), - ('methylcyclohexane', '2-methyl-6-propylnaphthalene'), - ('2,6-dimethylnaphthalene', '2-methyl-6-propylnaphthalene'), - ('2-naftanol', '2-methylnaphthalene'), - ('1,3,7-trimethylnaphthalene', '2-methyl-6-propylnaphthalene'), + ('1,3,7-trimethylnaphthalene', '2,6-dimethylnaphthalene'), + ('1,3,7-trimethylnaphthalene', '2-methyl-6-propylnaphthalene'), + ('1-butyl-4-methylbenzene', '2,6-dimethylnaphthalene'), + ('1-butyl-4-methylbenzene', '2-methyl-6-propylnaphthalene'), + ('1-butyl-4-methylbenzene', 'toluene'), + ('2,6-dimethylnaphthalene', '2-methyl-6-propylnaphthalene'), + ('2,6-dimethylnaphthalene', '2-methylnaphthalene'), + ('2,6-dimethylnaphthalene', '2-naftanol'), + ('2,6-dimethylnaphthalene', 'methylcyclohexane'), + ('2,6-dimethylnaphthalene', 'toluene'), + ('2-methyl-6-propylnaphthalene', '2-methylnaphthalene'), + ('2-methylnaphthalene', '2-naftanol'), + ('2-methylnaphthalene', 'methylcyclohexane'), + ('2-methylnaphthalene', 'toluene') ]) assert len(edge_ids) == len(ref) From e6595290fe64cf1070ea821a2a8beb0b9ad0f9aa Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Thu, 21 Sep 2023 10:49:07 +0200 Subject: [PATCH 25/28] more pep8 --- openfe/tests/setup/test_network_planning.py | 27 +++++++++++---------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/openfe/tests/setup/test_network_planning.py b/openfe/tests/setup/test_network_planning.py index 53411c577..0c0ceb555 100644 --- a/openfe/tests/setup/test_network_planning.py +++ b/openfe/tests/setup/test_network_planning.py @@ -246,6 +246,7 @@ def scorer(mapping): scorer=scorer ) + @pytest.fixture(scope='session') def minimal_redundant_network(toluene_vs_others, mst_num=2): toluene, others = toluene_vs_others @@ -303,19 +304,19 @@ def test_minimal_redundant_network(minimal_redundant_network): for edge in minimal_redundant_network.edges ) ref = sorted([ - ('1,3,7-trimethylnaphthalene', '2,6-dimethylnaphthalene'), - ('1,3,7-trimethylnaphthalene', '2-methyl-6-propylnaphthalene'), - ('1-butyl-4-methylbenzene', '2,6-dimethylnaphthalene'), - ('1-butyl-4-methylbenzene', '2-methyl-6-propylnaphthalene'), - ('1-butyl-4-methylbenzene', 'toluene'), - ('2,6-dimethylnaphthalene', '2-methyl-6-propylnaphthalene'), - ('2,6-dimethylnaphthalene', '2-methylnaphthalene'), - ('2,6-dimethylnaphthalene', '2-naftanol'), - ('2,6-dimethylnaphthalene', 'methylcyclohexane'), - ('2,6-dimethylnaphthalene', 'toluene'), - ('2-methyl-6-propylnaphthalene', '2-methylnaphthalene'), - ('2-methylnaphthalene', '2-naftanol'), - ('2-methylnaphthalene', 'methylcyclohexane'), + ('1,3,7-trimethylnaphthalene', '2,6-dimethylnaphthalene'), + ('1,3,7-trimethylnaphthalene', '2-methyl-6-propylnaphthalene'), + ('1-butyl-4-methylbenzene', '2,6-dimethylnaphthalene'), + ('1-butyl-4-methylbenzene', '2-methyl-6-propylnaphthalene'), + ('1-butyl-4-methylbenzene', 'toluene'), + ('2,6-dimethylnaphthalene', '2-methyl-6-propylnaphthalene'), + ('2,6-dimethylnaphthalene', '2-methylnaphthalene'), + ('2,6-dimethylnaphthalene', '2-naftanol'), + ('2,6-dimethylnaphthalene', 'methylcyclohexane'), + ('2,6-dimethylnaphthalene', 'toluene'), + ('2-methyl-6-propylnaphthalene', '2-methylnaphthalene'), + ('2-methylnaphthalene', '2-naftanol'), + ('2-methylnaphthalene', 'methylcyclohexane'), ('2-methylnaphthalene', 'toluene') ]) From f51c888bc4e3d5cb9e528d56fff7aaeff8fcd51f Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Fri, 22 Sep 2023 09:41:45 +0200 Subject: [PATCH 26/28] simplify test --- openfe/tests/setup/test_network_planning.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/openfe/tests/setup/test_network_planning.py b/openfe/tests/setup/test_network_planning.py index 0c0ceb555..a933d14cd 100644 --- a/openfe/tests/setup/test_network_planning.py +++ b/openfe/tests/setup/test_network_planning.py @@ -248,7 +248,7 @@ def scorer(mapping): @pytest.fixture(scope='session') -def minimal_redundant_network(toluene_vs_others, mst_num=2): +def minimal_redundant_network(toluene_vs_others): toluene, others = toluene_vs_others mappers = [BadMapper(), openfe.setup.atom_mapping.LomapAtomMapper()] @@ -259,7 +259,7 @@ def scorer(mapping): ligands=others + [toluene], mappers=mappers, scorer=scorer, - mst_num=mst_num + mst_num=2 ) return network @@ -292,8 +292,8 @@ def test_minimal_redundant_network_connectedness(minimal_redundant_network): def test_redundant_vs_spanning_network(minimal_redundant_network, minimal_spanning_network): # when setting minimal redundant network to only take one MST, it should have as many # edges as the regular minimum spanning network - assert len(minimal_spanning_network) == len( - minimal_redundant_network(mst_num=1)) + assert 2 * len(minimal_spanning_network.edges) == len( + minimal_redundant_network.edges) def test_minimal_redundant_network(minimal_redundant_network): From ee02143ee7bb3d5714bfd0acd610ed8e5ef42370 Mon Sep 17 00:00:00 2001 From: Jenke Scheen Date: Mon, 25 Sep 2023 09:25:05 +0200 Subject: [PATCH 27/28] rename duped fn name --- openfe/tests/setup/test_network_planning.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openfe/tests/setup/test_network_planning.py b/openfe/tests/setup/test_network_planning.py index a933d14cd..30dc78c01 100644 --- a/openfe/tests/setup/test_network_planning.py +++ b/openfe/tests/setup/test_network_planning.py @@ -296,7 +296,7 @@ def test_redundant_vs_spanning_network(minimal_redundant_network, minimal_spanni minimal_redundant_network.edges) -def test_minimal_redundant_network(minimal_redundant_network): +def test_minimal_redundant_network_edges(minimal_redundant_network): # issue #244, this was previously giving non-reproducible (yet valid) # networks when scores were tied. edge_ids = sorted( From dc666ea0a2b8bcce781599161309b45e036a3b18 Mon Sep 17 00:00:00 2001 From: Richard Gowers Date: Tue, 10 Oct 2023 09:28:10 +0100 Subject: [PATCH 28/28] Update ligand_network_planning.py --- openfe/setup/ligand_network_planning.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/openfe/setup/ligand_network_planning.py b/openfe/setup/ligand_network_planning.py index 385aee990..723c9edcf 100644 --- a/openfe/setup/ligand_network_planning.py +++ b/openfe/setup/ligand_network_planning.py @@ -237,7 +237,9 @@ def generate_minimal_redundant_network( mst_num: int = 2, ) -> LigandNetwork: """ - Plan a network with as few edges as possible with maximum total score, + Plan a network with a specified amount of redundancy for each node + + Creates a network with as few edges as possible with maximum total score, ensuring that every node is connected to two edges to introduce statistical redundancy.