Re: [myhdl-list] Conversion of class methods

[Angel E. <ang...@gm...>]

On Sat, Mar 17, 2012 at 6:33 AM, Christopher Felton
<chr...@gm...> wrote:
> I had put together a fairly generic (weak) MEP for class method
> conversion, http://www.myhdl.org/doku.php/meps:mep-108.  The following
> is a more in depth conversation/analysis of the items (I am aware of)
> that need to be handled to enable class method conversion.  The items
> discussed are:  The "self" argument, class attributes, and the extract
> hierarchy process.
>
> The MEP was only intended to handle class methods and not arbitrary
> conversion of classes.
>
> Handling the "self" argument
> -----------------------------
> If I naively send a class method to the conversion routines the method
> will fail to convert correctly.   It will fail because the "self"
> argument is treated as the first port (see the current MEP for an
> example).
>
> Performing a quick test by searching for "self" in the argument list and
> ignoring the self argument will render the object method convertible.
> Adding these changes to
>   _AnalyzeTopFuncVisitor.vist_FunctionDev(...) (_analyze.py)
> allows a top-level class method and buried methods to be converted.  I
> have prototyped and created some tests which can be viewed here,
> https://bitbucket.org/cfelton/myhdl-0.8-dev-containers/overview (clone
> of the myhdl 0.8 dev, see notes at the end).
>
> A couple *issues* with this approach!  Although the "self" argument is
> almost always used I don't believe its enforcement; the "first" argument
> can have any name.  A simple experiment demonstrates the object
> reference can be anything.
>
>   >>> class Foo():
>   ...     def __init__(this, self):
>   ...             print("New foo, this, self %s" % (self))
>   ...
>   >>> f = Foo('bar')
>   New foo, this, self bar
>   >>>
>
> A more generic identification would be required.  Ideally, you would
> like to identify the class method (distinguish the method from a
> function) and ignore the first argument, currently I do not know how
> this can be accomplished in the AST parsing (visiting).
>
> Assuming this technical hurdle can be overcome, the class method
> conversion would be possible.  But ... this only takes care of the
> "self" argument, in addition the class/objects attribute access and the
> extract hierarchy need to be addressed/discussed.
>
> Class attribute access
> -----------------------
> There are two types of attributes to consider.  The first a simple
> attribute (immutable?) which would simply hold a constant.  Example, the
> number of bits to have in a signal, etc.  Even these simple attributes
> would require changes.
>
> The bigger issue is accessing convertible types (Signal) that are
> referenced through the object's attributes.  This issue is a can of
> worms and I was not intending to address this "feature" with this MEP,
> but a separate MEP was created,
> http://www.myhdl.org/doku.php/meps:mep-107, to address general class
> attribute conversion.
>
> This brings up a couple questions:
>
>   1. Should an initial class method conversion be added with the
> limitation: no class attributes can be accessed?  (baby steps)
>
>   2. If no, then should the MEPs be changed so that 107 only addresses
> the other "containers" (if this is even desired) and 108 should include
> handling attributes?
>
> Extracting the hierarchy
> -------------------------
> While experimenting it was noticed that class methods were not
> identified in the _extractHeirarchy.  The _extractHeirarchy uses a
> different approach to determine the functions and generators that are to
> be converted.  The approach, using the "profile" functions does not
> identify the class methods, only functions (?).  This breaks some of the
> features (e.g. user-defined code).  It needs to be determined if the
> "profile" parsers can handle extracting the class methods or if the AST
> parser will need to be used (light-weight version of the AST to only
> extract hierarchy)?  Thanks to Wesley for providing the examples with
> the user-defined code.
>
> Closing remarks
> ----------------
> Note, this work is preliminary, I have not made enough progress to
> present a complete working solution.  This post is to inform the group
> what I have done and some issues I have run into and to solicit
> feedback.  Any comments or suggestions welcomed and appreciated.  I
> intent to hack away at this and try to resolve the above issues, time
> permitting.
>
> If you look at the commits in bitbucket be prepared.  One, I must have
> my editor to replace tabs with spaces.  In some cases more lines are
> modified than needed.  Two, I keep messing up mercurial and branches,
> extra commits to default that were not intended.  Both these mistakes
> make the commit diffs unusable.  At some point I will need a clean
> working clone, doh.
>
> References
> -----------
> [1] http://docs.python.org/tutorial/classes.html
>

Hi Christopher,

I wonder how you are messing up mercurial and branches. I find them
very easy and straight forward to use. Are you using plain mercurial
or a GUI such as TortoiseHg? I am one of the core TortoiseHg
developers, so do not hesitate to contact me if you need any help with
that. If you are not using it I'd love for you to give it a try. I
think it is quite helpful. If you are using it already and are having
problems with it I'd love to hear about them so that we can improve
things a little.

In any case, if you ever find that you committed to the wrong branch,
you can use the MQ extension to "qrefresh" your commit, changing the
branch at will.

Coming back to the important part of your message, you are right that
calling the first method argument "self" is just a convention, not a
requirement. Nevertheless I've never seen any python code that does
not call the first method argument "self". Thus I don't think you
should worry too much about it. I think that it would be safe to make
that a requirement for MyHDL if you do not find a better way to
differentiate between funcions and class methods.

As for your other questions, I find it a bit weird that the "signal
containers" MEP refers to "classes" as well. I don't think that
classes are normally considered containers even if strictly speaking
they are. Perhaps it is because they are so much more than that... So
my 0.02$ is that that part should be rolled into the MEP 108.

I also think that it is perhaps a good idea to go the "baby steps"
route and make a first version that does not handle class attributes.
I believe that any progress is good progress. Having this feature,
even in a limited form, may already prove useful, and it may even give
you hints on how to proceed further later on.

Cheers,

Angel