Skip to content

Rename module files in plotting folder - #554

Merged
nvaytet merged 8 commits into
mainfrom
module-renaming
Apr 21, 2026
Merged

nvaytet merged 8 commits into
mainfrom
module-renaming

Conversation

@nvaytet

@nvaytet nvaytet commented Apr 20, 2026 •

Copy link
Copy Markdown
Member

The plotting folder contains the most commonly used functions/modules.

Most files in that module contained a function with the same name as a file itself.
Because of the lazy-loader and how import mechanics work, we ran into the following problem:

import plopp as pp

# At this point, `pp.slicer` would point to the plotting.slicer.slicer()` function, as it should

from plopp.plotting.slicer import Slicer

# Now the name slicer has been reassigned
pp.slicer  # <-- points to the plotting.slicer module

So after importing the Slicer class, we can no longer call the pp.slicer function.

Here's what happens:

  1. plopp.__init__.py uses lazy.attach to lazily expose slicer (the function) from the plotting submodule at pp.slicer.

  2. When we from plopp.plotting.slicer import Slicer, Python imports the module plopp.plotting.slicer and, as part of the import machinery, sets it as an attribute on the plopp.plotting package: plopp.plotting.slicer = <module>.

  3. Now when the notebook later accesses pp.slicer, the lazy loader resolves it by doing from plopp.plotting import slicer. But plopp.plotting.slicer is now the module (set in step 2), not the function. So pp.slicer becomes the module, and calling it fails with TypeError: 'module' object is not callable.

So in this PR, we rename the files in these modules to avoid the name clashes.
There are probably other files in the Plopp codebase where this also happens, but for now we only rename the most used/important modules.
We can always fix more later.

I am hoping that very little code will be broken by this change, as most code or notebooks should be importing from plopp and not from plopp.plotting.slicer.

@jl-wynen jl-wynen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do the renamed files contain any public code that users should access via the full path (pp.slicer_plot.foo) or only code that is re-exported through __init__.py (pp.foo)? In the latter case, can we make the modules protected (_slicer.py) to simplify the UI, e.g., in autocompletions?

@nvaytet

nvaytet commented Apr 20, 2026

Copy link
Copy Markdown
Member Author

I think users should not have to import anything from the files in those modules, only from the __init__.py.

So are you suggesting instead of the renames that I did, to rename inspector.py to _inspector.py? etc

@jl-wynen

Copy link
Copy Markdown
Member

So are you suggesting instead of the renames that I did, to rename inspector.py to _inspector.py? etc

Yes

@nvaytet

nvaytet commented Apr 20, 2026

Copy link
Copy Markdown
Member Author

I tried running the tests in ess* packages, and built docs for essdiffraction and essreflectometry and nothing broke, so I am hopeful 🤞

@nvaytet
nvaytet merged commit 8bf9380 into main Apr 21, 2026
6 checks passed
@nvaytet
nvaytet deleted the module-renaming branch April 21, 2026 08:34
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.

2 participants