Skip to content
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

Fix import error in workflows base module (develop branch) #245

Closed
cadeduckworth opened this issue Apr 28, 2023 · 2 comments · Fixed by #243 or #257
Closed

Fix import error in workflows base module (develop branch) #245

cadeduckworth opened this issue Apr 28, 2023 · 2 comments · Fixed by #243 or #257
Assignees

Comments

@cadeduckworth
Copy link
Contributor

  • in base.py, change from mdpow.workflows import registry to import registry
  • or specify parent package and use relative import
@cadeduckworth
Copy link
Contributor Author

@orbeckst

This is from when I had problems with the imports recently. This issue is linked to #243.

I believe I tried the from . import registry method and the from mdpow.workflows import registry methods. I vaguely remember that mdpow is/was not allowing imports from mdpow.workflows or mdpow.workflows.dihedrals, etc. Is it possible that I did not add something required for this to work?

orbeckst added a commit that referenced this issue Jul 2, 2023
- fix #245
- avoid circular imports involving the workflows.registry
@orbeckst
Copy link
Member

orbeckst commented Jul 2, 2023

Have a look at PR #257 — seems to work.

There shouldn't be any restrictions on imports, there's nothing in MDPOW that messes with Python's import machinery.

I think we had some circular imports involving the registry (base imports registry, dihedrals imports base, registry imports dihedrals) and these circular imports are generally bad because they can lead to memory leaks, but in the PR I broke the import by importing the registry inside base inside a function so it will only be run when needed. At this time, Python figures out that registry is already in sys.modules and will just use that without trying to go to the actual file. All is good 🦄 🌈 .

@orbeckst orbeckst self-assigned this Jul 2, 2023
orbeckst added a commit that referenced this issue Jul 4, 2023
* add RDKit Mol object to dihedral analysis plots
* add tests, and close  #238
* add svgutils and cairosvg methods to plot svg mol object
* reimplement DF input option and fix most tests to reflect name changes and altered function definitions
* add svgutils and cairosvg to dependencies, install, requirements lists, remove broken test, add reminder to update func list in docs
* split plot_violins into new build_svg function
* change, better function names for dihedrals workflow module
* docs and cleanup, plot width docs, dict comprehension for ab_pairs
* intersphinx mapping
* tests: new fixtures and tests for bond_indices and ab_pairs
* tests: new fixtures and tests for bond_indices and ab_pairs, skip 3.7
* test_build_universe method
* confirm build universe test
* rewrite docs to cover new functions and kwarg changes
* fix tests to accommodate kwarg updates in dihedrals module
* explanation of why figdir is a kwarg at top level of dihedrals module but a positional argument elsewhere - workflows base **kwargs, issue #244, see in-line comment in dihedrals.py
* temporary fix for figdir issue which should currently be a positional argument, but would require redundant rewrite of workflows base module, pending issue #244
* upcoming CHANGES
* remove dafault scope specification for defined functions
* reimplement try/except method for rdkit conversion topology element guessing
* generate combined plots pdf for automated dihedral analysis
* updates for implementation of pypdf in workflows dihedrals module: CHANGES, testing environment, requirements, sphinx source configuration
* documentation for dihedral_violins function in workflows dihedrals module
* documentation for get_paired_indices function in workflows dihedrals module
* documentation and kwarg definition for get_paired_indices function and ab_pairs dictionary object in workflows dihedrals module
* kwarg definition for plot_title for dihedral_violins function in workflows dihedrals module
* move in-line comments explaining figdir kward for workflows dihedrals module
* reorganize kwargs for plot_dihedral_violins in top-level automated_dihedral_analysis function call in workflows dihedrals module
* add assert method to make figdir kwarg required in workflows dihedrals module
* change MDA guess_atom_element to MDA guess_types for RDKit conversion in workflows dihedrals module
* fix registry import error for workflows base, close #245
* remove guess_atom_element import
* reimplement assert figdir reuired for workflows dihedrals module
* add pypdf to setup.py install_requires for dihedrals workflow
* change imports to follow PEP 8
* modify dihedrals workflow docs to explain figdir kwarg requirement
* use first solvent specified to build MDAnalysis Universe
* modify single solvent plotting method, add solvent count assertion
* comment expected fixture scope changes, reference issue #235
* remove solute.unwrap, not needed
* reference issue #260 to fix jupyter notebook figure output
* finalize single solvent figure modifications and add test

---------

Co-authored-by: Oliver Beckstein <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants