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 dir() method #20321

Draft
wants to merge 13 commits into
base: develop
Choose a base branch
from
Draft

Fix dir() method #20321

wants to merge 13 commits into from

Conversation

markcmiller86
Copy link
Member

@markcmiller86 markcmiller86 commented Mar 21, 2025

Description

Resolves #19264

  • Adds a custom __dir__() method to our python objects which lists all methods in the _methods[] table (except __dir__ and Notify) as well as all non-internal data members
    • For an example of an object that has internal members, look at the dir() method in PySpreadsheetAttributes.C
  • Adds _methods table pointer to the tp_methods slot of the object header
  • Changes Notify to METH_NOARGS in the _methods[] table
  • Increases _NMETH by one (which causes all associated .h files to change too)
  • Improves behavior of apropos

Type of change

  • Bug fix~~
  • [~~ ] New feature~~
  • [ ] Documentation update
  • [ ] Other

How Has This Been Tested?

Reminders:

  • Please follow the style guidelines of this project.
  • Please perform a self-review of your code before submitting a PR and asking others to review it.
  • Please assign reviewers (see VisIt's PR procedures for more information).

Checklist:

  • I have commented my code where applicable.~~
  • I have updated the release notes.~~
  • I have made corresponding changes to the documentation.~~
  • I have added debugging support to my changes.~~
  • I have added tests that prove my fix is effective or that my feature works.~~
  • I have confirmed new and existing unit tests pass locally with my changes.~~
  • I have added new baselines for any new tests to the repo.~~
  • I have NOT made any changes to protocol or public interfaces in an RC branch.~~

@markcmiller86
Copy link
Member Author

I've discovered from other changes I need to make here. isinstance(x, str) was generating a ValueError exception due to no __class__ attribute. I think the solution is that we need to switch from getattr/setattr to getattro/setattro. I am testing on one object and it seems to be behaving better.

@markcmiller86
Copy link
Member Author

markcmiller86 commented Mar 27, 2025

@visit-dav/visit-developers and @cyrush can you please have a look at the direction this is headed and lemme know what you think. There are 3 key things to look at here...

  • Py2and3support.h macros for handling python objects. I've tried a few different approaches here and am settling on this. That said, there is a far nicer approach if we are willing to require C++ designated initializers (a C++ 20 standard but available far sooner in many compilers).
    • // Mark C. Miller, Wed Mar 26 18:58:25 PDT 2025
      // These macros are used within the definition of VISIT_PY_TYPE_OBJ to help
      // build a block of code representing the default set of .tp_xxx slot methods
      // our standard objects need to define. Its likely overly cautious but the reason
      // for the funky retval incrimenting here is to ensure there is no way any compiler
      // optimizations could maybe decide the function in which this code block appears
      // could ever be optimized away because its return value could maybe be computed at
      // compile time.
      //
      // We make prolific use of the CPP paste operator (##) here to enforce naming
      // consistency.
      //
      #define VISIT_PY_TYPE_OBJ_SLOT1(VSObjName, SLOT) \
      Py##VSObjName##Type.tp_##SLOT = Py##VSObjName##_##SLOT; \
      retval += ((void*) Py##VSObjName##Type.tp_##SLOT != (void*)0)
      #define VISIT_PY_TYPE_OBJ_SLOT2(VSObjName, SLOT, FUNC) \
      Py##VSObjName##Type.tp_##SLOT = Py##VSObjName##_##FUNC; \
      retval += ((void*) Py##VSObjName##Type.tp_##SLOT != (void*)0)
      //
      // For non-standard objects, the correct approach is to #undef this macro and
      // then re-define it to what is needed AHEAD of invoking VISIT_PY_TYPE_OBJ().
      //
      #define VISIT_PY_TYPE_OBJ_TP_SLOTS(VSObjName) \
      VISIT_PY_TYPE_OBJ_SLOT1(VSObjName, dealloc); \
      VISIT_PY_TYPE_OBJ_SLOT2(VSObjName, repr, str); \
      VISIT_PY_TYPE_OBJ_SLOT1(VSObjName, str); \
      VISIT_PY_TYPE_OBJ_SLOT1(VSObjName, getattro); \
      VISIT_PY_TYPE_OBJ_SLOT1(VSObjName, setattro); \
      VISIT_PY_TYPE_OBJ_SLOT1(VSObjName, richcompare); \
      VISIT_PY_TYPE_OBJ_SLOT1(VSObjName, methods)
      // 2023/01/27 CYRUSH Note:
      // tp_print / tp_vectorcall_offset slot is not used by python 3.0 - 3.7
      // it should not be used for python 3.8 > unless Py_TPFLAGS_HAVE_VECTORCALL
      // is passed, but we choose to always set to 0
      //
      // Also we use VPY_STR to implement both str() and repr(), as this
      // preserves the Python 2 cli behavior to echo object contents via repr()
      //
      // Mark C. Miller, Mon Mar 24 16:07:55 PDT 2025
      // Adjust VISIT_PY_TYPE_OBJ to initialize our python objects in two steps.
      // The first is an *assignment* of the PyTypeObject struct with mostly zero
      // entries and other critical boilerplate stuff. The second is the assignment
      // to a dummy static variable of the result of calling a static initialization
      // function for the type. It is in that function where the remaining .tp_xxx
      // slots are set up according to the (above) VISIT_PY_TYPE_OBJ_TP_SLOTS macro.
      // To override the default behavior, simply #undef VISIT_PY_TYPE_OBJ_TP_SLOTS
      // and then re-define it to what is needed before invoking VISIT_PY_TYPE_OBJ.
      //
      // We make prolific use of the CPP paste operator (##) here to enforce naming
      // consistency.
      //
      #define VISIT_PY_TYPE_OBJ(VSObjName) \
      static PyTypeObject Py##VSObjName##Type = \
      { \
      PyVarObject_HEAD_INIT(&PyType_Type, 0) \
      #VSObjName, /* tp_name */ \
      sizeof(Py##VSObjName##Object),/* tp_basicsize */ \
      0, /* tp_itemsize */ \
      0, /* tp_dealloc */ \
      0, /* tp_print (py2 and < py3.8)... */ \
      /* ...or tp_vectorcall_offset (>py3.8 */ \
      0, /* tp_getattr */ \
      0, /* tp_setattr */ \
      0, /* tp_reserved */ \
      0, /* tp_repr */ \
      0, /* tp_as_number */ \
      0, /* tp_as_sequence */ \
      0, /* tp_as_mapping */ \
      0, /* tp_hash */ \
      0, /* tp_call */ \
      0, /* tp_str */ \
      0, /* tp_getattro */ \
      0, /* tp_setattro */ \
      0, /* tp_as_buffer */ \
      PY_VISIT_TPFLAGS_DEFAULT, /* tp_flags */ \
      Py##VSObjName##_purpose, /* tp_doc */ \
      0, /* tp_traverse */ \
      0, /* tp_clear */ \
      0, /* tp_richcompare */ \
      0, /* tp_weaklistoffset */ \
      0, /* tp_iter */ \
      0, /* tp_iternext */ \
      0, /* tp_methods */ \
      0, /* tp_members */ \
      0, /* tp_getset */ \
      0, /* tp_base */ \
      0, /* tp_dict */ \
      0, /* tp_descr_get */ \
      0, /* tp_descr_set */ \
      0, /* tp_dictoffset */ \
      0, /* tp_init */ \
      0, /* tp_alloc */ \
      0, /* tp_new */ \
      0, /* tp_free */ \
      0, /* tp_is_gc */ \
      0, /* tp_bases */ \
      0, /* tp_mro */ \
      0, /* tp_cache */ \
      0, /* tp_subclasses */ \
      0, /* tp_weaklist */ \
      0, /* tp_del */ \
      0 /* tp_version_tag */ \
      PyVarObject_TAIL \
      }; \
      static int VSObjName##_init(void) { \
      static int retval = 0; \
      VISIT_PY_TYPE_OBJ_TP_SLOTS(VSObjName); \
      return retval; \
      } \
      static int VSObjName##_initialized = VSObjName##_init()
  • An example standard object (fully code-gened)...
    • //
      // Initialize the python object type structure with default values.
      // If you need to do something custom, #undef VISIT_PY_TYPE_OBJ_TP_SLOTS,
      // and then re-define it here AHEAD of VISIT_PY_TYPE_OBJ. For examples, look
      // in src/avt/PythonFilters and in src/visitpy/common/Py2and3Support.h
      //
      VISIT_PY_TYPE_OBJ(PseudocolorAttributes);
  • An example of a custom object (manually written)...
    • // Re-define tp slots for this custom object
      #undef VISIT_PY_TYPE_OBJ_TP_SLOTS
      #define VISIT_PY_TYPE_OBJ_TP_SLOTS(VSObjName) \
      VISIT_PY_TYPE_OBJ_SLOT2(VSObjName, doc, purpose); \
      VISIT_PY_TYPE_OBJ_SLOT1(VSObjName, dealloc); \
      VISIT_PY_TYPE_OBJ_SLOT1(VSObjName, getattro); \
      VISIT_PY_TYPE_OBJ_SLOT1(VSObjName, setattro); \
      VISIT_PY_TYPE_OBJ_SLOT1(VSObjName, str); \
      VISIT_PY_TYPE_OBJ_SLOT1(VSObjName, richcompare); \
      VISIT_PY_TYPE_OBJ_SLOT1(VSObjName, as_number); \
      VISIT_PY_TYPE_OBJ_SLOT1(VSObjName, methods)
      VISIT_PY_TYPE_OBJ(View3DAttributes);

Copy link
Contributor

@biagas biagas left a comment

Choose a reason for hiding this comment

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

The setattro/getattro will still allow custom logic for handling old settings? (As is currently done for VolumePlot)?

You will be verifying the test suite, correct?

{
// Contract is stored in a a ref ptr, so it will clean itself up.
// but ?
}


// ****************************************************************************
// Function: Contract_getattr
// Function: Contract_getattro
Copy link
Contributor

Choose a reason for hiding this comment

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

Might want to change the function names in the comments to match new names.

@markcmiller86
Copy link
Member Author

The setattro/getattro will still allow custom logic for handling old settings? (As is currently done for VolumePlot)?

Not sure what you mean. set/getattro is the python 3 way to do set/getattr. Same function bodies. Slightly different call signatures.

You will be verifying the test suite, correct?

Well, if I must 🤔.

@biagas
Copy link
Contributor

biagas commented Mar 27, 2025

The setattro/getattro will still allow custom logic for handling old settings? (As is currently done for VolumePlot)?

Not sure what you mean. set/getattro is the python 3 way to do set/getattr. Same function bodies. Slightly different call signatures.

We allow pre/post fixing code to set/getattr to allow processing old atts during a 'transition' period while people adjust their scripts. Similar to 'ProcessOldVersions' method for the C-Atts when reading configs/session files.
The logic is specified in the .code file and GeneratePython ensures it gets written.
Here's the VolumePlot.code section as an example:
https://github.com/visit-dav/visit/blob/ed72b88894b6c0fccccb6696f22ccefd1eef1983/src/plots/Volume/VolumeAttributes.code#L628C1-L671C1

Just want to make sure that this logic will still work after your changes (probably need to adjust the function name).

@markcmiller86
Copy link
Member Author

I think the answer is that whatever GeneratePython.h did for get/setattr, it now does for get/setattro. Only the emitted function names and call signatures have changed.

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.

CLI: dir is broken for State attributes (python 3.9.18)
2 participants