Skip to content

Hua work miscs #15

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.

Sign up for GitHub

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

Merged
zaihuaji merged 3 commits into from
Mar 25, 2026
Merged

Hua work miscs #15

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
04046a5
Instantiate class to read instance-level OPTS and ALIAS in load_opts_…
Mar 25, 2026
1c8ec8a
save for testing
Mar 25, 2026
10bfbda
rst_templates
Mar 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .claude/worktrees/sad-shockley
Open in desktop
Submodule sad-shockley added at 093dc7
1 change: 1 addition & 0 deletions MANIFEST.in
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
include src/rda_python_miscs/*.usg
include src/rda_python_miscs/rst_templates/*.temp
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ build-backend = "setuptools.build_meta"

[project]
name = "rda_python_miscs"
version = "2.0.3"
version = "2.0.4"
authors = [
{ name="Zaihua Ji", email="zji@ucar.edu" },
]
Expand Down
40 changes: 21 additions & 19 deletions src/rda_python_miscs/pg_rst.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1119,9 +1119,10 @@ def load_opts_alias(self, docname):

Resolution order for OPTS / ALIAS:

1. Module-level ``OPTS`` / ``ALIAS`` attributes.
2. The first class *defined in that module* that carries both ``OPTS``
and ``ALIAS`` as class-level attributes.
1. The first class *defined in that module* that carries ``OPTS``
as a class-level attribute.
2. Module-level ``OPTS`` / ``ALIAS`` attributes (fallback when no
qualifying class is found).

``ALIAS`` is optional; an empty dict is returned when not found.

Expand Down Expand Up @@ -1155,25 +1156,26 @@ def load_opts_alias(self, docname):
# Derive ORIGIN from the module's own file path.
origin = op.dirname(op.abspath(mod.__file__))

# 1. Try module-level attributes first.
opts = getattr(mod, 'OPTS', None)
alias = getattr(mod, 'ALIAS', None)

# 2. Fall back to the first class in the module that owns both.
if opts is None or alias is None:
for _, obj in inspect.getmembers(mod, inspect.isclass):
if obj.__module__ == modname:
cls_opts = getattr(obj, 'OPTS', None)
cls_alias = getattr(obj, 'ALIAS', None)
if cls_opts is not None:
if opts is None: opts = cls_opts
if alias is None: alias = cls_alias
break
# 1. Find the first class defined in this module and read OPTS / ALIAS from it.
cls = next(
(obj for _, obj in inspect.getmembers(mod, inspect.isclass)
if obj.__module__ == modname),
Comment on lines +1161 to +1162

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

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

inspect.getmembers(mod, inspect.isclass) returns members sorted by name, not in source-definition order, so this doesn't actually select the “first class defined in that module” as the docstring states. If definition order matters, iterate over mod.__dict__ (insertion-ordered) and filter classes from there, or adjust the wording to match the actual behavior.

Suggested change
(obj for _, obj in inspect.getmembers(mod, inspect.isclass)
if obj.__module__ == modname),
(
obj
for obj in mod.__dict__.values()
if inspect.isclass(obj) and obj.__module__ == modname
),

Copilot uses AI. Check for mistakes.
None,
)

if cls is not None:
obj = cls()
opts = getattr(obj, 'OPTS', None)
alias = getattr(obj, 'ALIAS', None)
Comment on lines +1167 to +1169

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

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

This instantiates the first class found via cls(). That can raise TypeError if the class requires constructor arguments, and it can trigger side effects in __init__. The docstring also says OPTS/ALIAS are class-level attributes, so it should read from the class (getattr(cls, ...)) rather than an instance.

Suggested change
obj = cls()
opts = getattr(obj, 'OPTS', None)
alias = getattr(obj, 'ALIAS', None)
# Read class-level attributes directly from the class to avoid
# instantiating it (which may require constructor arguments or
# trigger side effects in __init__).
opts = getattr(cls, 'OPTS', None)
alias = getattr(cls, 'ALIAS', None)

Copilot uses AI. Check for mistakes.
else:
# 2. Fall back to module-level attributes when no class is found.
opts = getattr(mod, 'OPTS', None)
alias = getattr(mod, 'ALIAS', None)
Comment on lines +1166 to +1173

Copilot AI Mar 25, 2026

Copy link

Choose a reason for hiding this comment

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

If a module defines at least one class but that first class doesn't have OPTS, this code will not fall back to module-level OPTS even though the docstring describes module-level as a fallback when no qualifying class is found. Consider continuing to search for a class that defines OPTS, or falling back to module-level when opts is still None after checking the class.

Copilot uses AI. Check for mistakes.

if opts is None:
self.pglog(
"Module '{}' does not define OPTS (checked module level and "
"all classes defined in the module)".format(modname),
"Module '{}' does not define OPTS (checked class and "
"module level)".format(modname),
self.LGWNEX,
)

Expand Down
Loading