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

Feedback on using ltk.Model #19

Open
Neon22 opened this issue Jan 8, 2025 · 21 comments
Open

Feedback on using ltk.Model #19

Neon22 opened this issue Jan 8, 2025 · 21 comments
Assignees

Comments

@Neon22
Copy link

Neon22 commented Jan 8, 2025

MVP Feedback:

In trying to migrate to using the MVP (ltk.Model class) I have come across some issues.
I will lay them out here in the hopes that it might help in future development.

  1. To get around the potential memory leak in widget() I have successfully been using a global var. The MVP approach looks like it may supersede the need for the recently added widget()

  2. I am also using a global var to make easy ref to the following reactive component. This is working well.
    I am also using a global to deal with rippling updates, if I update two or more reactive variables. This is working well.

I.e.

g_RC_values = None  # Hold reactive Model
g_inhibit_update = False  # prevent rippling updates

class Reactive_component(ltk.Model):
    """
    These sets of triplet variables update each other. Doing unit conversions
    """
    #  Length
    Len_measure: str = "250yd"
    Len_units: str = "yd"
    Len_altlabel: str = "229m"
    #
    Sk_wraps: str = "20wpi"
    Sk_units: str = "wpi"
    Sk_altlabel: str = "7.9wpcm"

    def changed(self, name, value):
        """
        Called whenever a UI element is changed by user.
        Also invoked for internal changes
        - g_inhibit_update is used to prevent needless rippling
        """
        global g_inhibit_update
        print("Change requested:",name,value)
        if not g_inhibit_update:
            print("Changing:",name,value)
            g_inhibit_update = True  # inhibit rippling calls while we update
            # Skein Length
            if name in ["Len_measure", "Len_units"]:
                vars = ["Len_measure", "Len_units", "Len_altlabel", ["250","yd","m"]]
                if name == "Len_measure":
                    self.update_primary(value, *vars)
                elif name == "Len_units":
                    self.recalc_units(value, *vars)
            # Skein units
            if name in ["Sk_wraps", "Sk_units"]:
                vars = ["Sk_wraps", "Sk_units", "Sk_altlabel", ["20","wpi","wpcm"]]
                if name == "Sk_wraps":
                    self.update_primary(value, *vars)
                elif name == "Sk_units":
                    self.recalc_units(value, *vars)
            # reset ripple updates
            g_inhibit_update = False
        else:
            print(" -Skipping change")

if __name__ == "__main__":
    g_RC_values = Reactive_component()
    w = Widget(g_RC_values)
    widget = w.create()
    widget.appendTo(ltk.window.document.body)
  1. autocreation of instance variables:
  • The reason that I am using the names of the variables rather than the variables themselves is because of a side effect of ltk.Model using class variables.
    Specifically:
  • I can set a class var just fine with self.Len_measure = "foo"
  • but if I pass self.Len_measure through a function header and try to assign it, then an instance variable is created and the class var is not updated.
    E.g.
# in changed()
self.update_myvar(self.Len_measure)

def update_myvar(self, myvar):
   myvar = "Foo"
   # The iv is created and updated but the classvar is unchanged

So to get around this I send the name of the classvar through (as in changed() above and do a more complicated operation like so:

def update_primary(self, value, primary_var, unit_var, alt_var,
                      defaults=["250","yd","m"]):
       """
       Update the primary, units, alts on basis of input.
       value can have units or not. Use as supplied and update alt.
       Use defaults if missing or error
       - Have to use self.__setattr__("varname", value) because its class var !
       """
       num, unit = parse_units(value)
       if num and num > -1:
           # got a valid measure but maybe no units
           if not unit:
               # use on-screen units
               units = self.__dict__[unit_var]
               self.__setattr__(primary_var, f"{format_nice(num)}{units}")
               if units in metric:
                   num = num * convert_factors[units]
               self.__setattr__(alt_var, convert_imp(num, unit_swaps[units], self.Len_units=="in"))
           else:  # unit from measure
               if unit not in convert_factors.keys():
                   unit = defaults[1] #"yd"
               self.__setattr__(unit_var, unit)
               usnum,_ = parse_to_US(value)
               self.__setattr__(primary_var, convert_imp(usnum, unit))
               self.__setattr__(alt_var, convert_imp(usnum, unit_swaps[unit]))
       else:  # not valid so set default
           self.__setattr__(primary_var, defaults[0]+defaults[1])
           self.__setattr__(unit_var, defaults[1])
           self.__setattr__(alt_var, convert_imp(int(defaults[0]), defaults[2]))

In short:
I am using self.__setattr__(primary_var, newvalue) instead of primary_var = newvalue because its a classvar.

So I am ok with my solution and its as neat as I can work out how to make it (by using the expanding *vars parameter).

But I wonder if there is another way to architect it so it works differently.
Working example here:

Cheers...

@Neon22
Copy link
Author

Neon22 commented Jan 27, 2025

Additional related information:
Can't use += kinds of operators on ModelAttributes

When changing a ModelAttribute we can do this:

class Reactive_component(ltk.Model):
    Drawin = 12

def changed(self, name, value):
   self.Drawin = self.Drawin + 2  # works
   self.Drawin.set_value(self.Drawin.get_value() + 2)  # also works
   # but
   self.Drawin += 2  # fails

Error message is:

Error calling function from JavaScript
  <closure <function set_model_value at 0x542a0> at 542c0, n_closed=2
     <cell 539b0
        <Checkbox object at 539b0>>
    <cell 53290 False> >:
 can't convert NoneType to int

This may all be classvar vs instancevar problems and not so easily solved.

@Neon22
Copy link
Author

Neon22 commented Jan 27, 2025

Also I have been caught out a few times as ModelAttribute has a __str__ function which means it prints as a string which sometimes works and sometimes doesn't if I am not careful when I use it. So been trying to manually add a lot of str() or f"{}" when I mean to get its value. Maybe we could do with an __repr__ but not an __str__ ??

@Neon22
Copy link
Author

Neon22 commented Jan 28, 2025

I have not succeeded in making a component style object using the Model class.
Because its a class variable I have to make a new class for each instance of a component. This is impractical.

A good solution would be to move to instance variables. I wonder if this is at all possible?

@Neon22
Copy link
Author

Neon22 commented Jan 31, 2025

A small side note - in Chrome - when using an ltk.Input. We are no longer adding a listener, because reactive is working.
But - mouse input - while it works in all cases, does not automatically call the reactive update until the user leaves the field.

IWBNI mouse scrolling on numeric inputs also did a live reactive update.

@laffra laffra self-assigned this Feb 4, 2025
@laffra
Copy link
Collaborator

laffra commented Feb 4, 2025

Also I have been caught out a few times as ModelAttribute has a __str__ function which means it prints as a string which sometimes works and sometimes doesn't if I am not careful when I use it. So been trying to manually add a lot of str() or f"{}" when I mean to get its value. Maybe we could do with an __repr__ but not an __str__ ??

Not sure what you mean here. There is a difference between str and repr in Python. I added the str for completeness. It calls str() on the underlying value. When would that fail?

@laffra
Copy link
Collaborator

laffra commented Feb 4, 2025

The class attributes for ltk.Model subclasses act as a template for creating instance. You do not directly set new values on the class fields, but set them on the instances. I updated the example in the Kitchensink to make this more obvious, hopefully. Does that clear up the confusion? See https://pyscript.github.io/ltk/?tab=3

@laffra
Copy link
Collaborator

laffra commented Feb 4, 2025

Overrides in ltk.ModelAttribute iadd and similar methods were missing call to set_value. Updated the implementation and the sample to show working code.

@laffra
Copy link
Collaborator

laffra commented Feb 4, 2025

I have not succeeded in making a component style object using the Model class. Because its a class variable I have to make a new class for each instance of a component. This is impractical.

A good solution would be to move to instance variables. I wonder if this is at all possible?

The Model IS instance based. The new example should clarify this a bit better?
https://pyscript.github.io/ltk/?tab=3

@Neon22
Copy link
Author

Neon22 commented Feb 4, 2025

Ahh ok. I thought it was on the class variables because I could not set instance variables at all. (you have clarified why above)
Looking at Tab3 today.
Thanks Chris, sorry for the confusion.

wrt the __str__:
If the value being held is not a string then I get caught out when I should be using .value or an fstring to get the (say) integer.
My request for __repr__ is purely about debugging. If there is no __str__ method then it falls back to the repr and this would show what instance I was looking at - which is "useful".

Possibly all my confusion will be cleaned up when I look again today.
Cheers...

@laffra
Copy link
Collaborator

laffra commented Feb 4, 2025

Do you have an example for the str problem? I still don't understand it 🤓 .

@Neon22
Copy link
Author

Neon22 commented Feb 6, 2025

Brilliant - now working very well.
I think my str issues were all to do with the way I was reading/writing the class variables using self.__setattr__.
In any case - there is no issue when I use the library "properly" as designed :)

Got an approach working for fully encapsulating a UI component in a single python file (well except putting the css into its own block. Personally a separate css file still seems like a very good/manageable solution.)

@laffra
Copy link
Collaborator

laffra commented Feb 6, 2025

This is great to hear.

@Neon22
Copy link
Author

Neon22 commented Feb 7, 2025

OK. I am in the process of trying to make single file components.

I think it would be useful to have a helper function for css use.
Is something like this possible:

if __name__ == "__main__":
    w= My_new_component()
    widget = w.create()
    widget.appendTo(ltk.window.document.body)
    # 
    print(w.css_report())

Where .css_report() printed out a list/dictionary of:

  • names of id attributes used (each.attr("id", foo))
  • names of all classes added in .addClass()
  • whether or not .css() was used
  • Perhaps somomething like:
  • {"ids": [], "classes": [], ".css": [] }

This would help me to identify and create a css file for this component.

The reasoning behind this is to make use of the __main__ python approach to debug/visualise a component while developing it, or examining a component you think you want to include.

I am thinking at the moment that each "single file component" is really two files (assuming no required imports).
The py file and its associated css file. Presumably with the same name.

Then users can choose to manually amalgamate the css for efficiency but when initially building you'd just end up loading a few css files.

Thoughts ??

@laffra
Copy link
Collaborator

laffra commented Feb 7, 2025 via email

@laffra
Copy link
Collaborator

laffra commented Feb 7, 2025

See LTK v.2.22. The inspector now shows classes and css.

@Neon22
Copy link
Author

Neon22 commented Feb 7, 2025

OK. Thanks but :)

  • I am getting huge amounts of css even when over an Input. It looks like all the webkit etc stuff as we might expect.
    • but its not mentioning the classes I added using .addClass()
  • However its not possible for me to work out (if I'm over the top level element) the three pieces of info I need:
    • I.e. all ids in use and all the classes I specified. I need this to check I have got entries for them all in my css file.
  • Also its adding stuff to the dom somehow here's a screenshot:
    • The extra empty boxes keep appearing as I move the mouse around

Image

So its giving me this when I'm over an Input: (which can't fit on screen)

An LTK Python widget of class Input
Wraps an HTML element of type <input>
classes = ['ltk-input']
tag = input
Run with PyOdide to show where this widget was created
0 children
css:
{
    "alignSelf": "center",
    "blockSize": "28px",
    "border": "1px solid rgb(225, 225, 225)",
    "borderBlock": "1px solid rgb(225, 225, 225)",
    "borderBlockColor": "rgb(225, 225, 225)",
    "borderBlockEnd": "1px solid rgb(225, 225, 225)",
    "borderBlockEndColor": "rgb(225, 225, 225)",
    "borderBlockEndStyle": "solid",
    "borderBlockEndWidth": "1px",
    "borderBlockStart": "1px solid rgb(225, 225, 225)",
    "borderBlockStartColor": "rgb(225, 225, 225)",
    "borderBlockStartStyle": "solid",
    "borderBlockStartWidth": "1px",
    "borderBlockStyle": "solid",
    "borderBlockWidth": "1px",
    "borderBottom": "1px solid rgb(225, 225, 225)",
    "borderBottomColor": "rgb(225, 225, 225)",
    "borderBottomStyle": "solid",
    "borderBottomWidth": "1px",
    "borderColor": "rgb(225, 225, 225)",
    "borderInline": "1px solid rgb(225, 225, 225)",
    "borderInlineColor": "rgb(225, 225, 225)",
    "borderInlineEnd": "1px solid rgb(225, 225, 225)",
    "borderInlineEndColor": "rgb(225, 225, 225)",
    "borderInlineEndStyle": "solid",
    "borderInlineEndWidth": "1px",
    "borderInlineStart": "1px solid rgb(225, 225, 225)",
    "borderInlineStartColor": "rgb(225, 225, 225)",
    "borderInlineStartStyle": "solid",
    "borderInlineStartWidth": "1px",
    "borderInlineStyle": "solid",
    "borderInlineWidth": "1px",
    "borderLeft": "1px solid rgb(225, 225, 225)",
    "borderLeftColor": "rgb(225, 225, 225)",
    "borderLeftStyle": "solid",
    "borderLeftWidth": "1px",
    "borderRight": "1px solid rgb(225, 225, 225)",
    "borderRightColor": "rgb(225, 225, 225)",
    "borderRightStyle": "solid",
    "borderRightWidth": "1px",
    "borderStyle": "solid",
    "borderTop": "1px solid rgb(225, 225, 225)",
    "borderTopColor": "rgb(225, 225, 225)",
    "borderTopStyle": "solid",
    "borderTopWidth": "1px",
    "borderWidth": "1px",
    "display": "block",
    "font": "22.4px Arial",
    "fontSize": "22.4px",
    "height": "28px",
    "inlineSize": "60px",
    "minBlockSize": "auto",
    "minHeight": "auto",
    "minInlineSize": "36px",
    "minWidth": "36px",
    "padding": "3px",
    "paddingBlock": "3px",
    "paddingBlockEnd": "3px",
    "paddingBlockStart": "3px",
    "paddingBottom": "3px",
    "paddingInline": "3px",
    "paddingInlineEnd": "3px",
    "paddingInlineStart": "3px",
    "paddingLeft": "3px",
    "paddingRight": "3px",
    "paddingTop": "3px",
    "perspectiveOrigin": "30px 14px",
    "placeSelf": "center auto",
    "transformOrigin": "30px 14px",
    "webkitAlignSelf": "center",
    "webkitBorderAfter": "1px solid rgb(225, 225, 225)",
    "webkitBorderAfterColor": "rgb(225, 225, 225)",
    "webkitBorderAfterStyle": "solid",
    "webkitBorderAfterWidth": "1px",
    "webkitBorderBefore": "1px solid rgb(225, 225, 225)",
    "webkitBorderBeforeColor": "rgb(225, 225, 225)",
    "webkitBorderBeforeStyle": "solid",
    "webkitBorderBeforeWidth": "1px",
    "webkitBorderEnd": "1px solid rgb(225, 225, 225)",
    "webkitBorderEndColor": "rgb(225, 225, 225)",
    "webkitBorderEndStyle": "solid",
    "webkitBorderEndWidth": "1px",
    "webkitBorderStart": "1px solid rgb(225, 225, 225)",
    "webkitBorderStartColor": "rgb(225, 225, 225)",
    "webkitBorderStartStyle": "solid",
    "webkitBorderStartWidth": "1px",
    "webkitLogicalHeight": "28px",
    "webkitLogicalWidth": "60px",
    "webkitMinLogicalHeight": "auto",
    "webkitMinLogicalWidth": "36px",
    "webkitPaddingAfter": "3px",
    "webkitPaddingBefore": "3px",
    "webkitPaddingEnd": "3px",
    "webkitPaddingStart": "3px",
    "webkitPerspectiveOrigin": "30px 14px",
    "webkitTransformOrigin": "30px 14px",
    "width": "60px"
}

When what I want to know is:

  • for this widget - what #ids are in use, what classnames are in use, did the author use any direct .css() methods when they made it - because probably I should change that to a classname instead so the Component() will be more portable.

Like this {"ids": [], "classes": [], ".css": [] }

@Neon22
Copy link
Author

Neon22 commented Feb 7, 2025

So for my example here:

I would like to know:
{"ids": [], "classes": ["Item", "paramtitle", "measure", "short", "param", "altlabel", "noborder"], ".css": True }

I agree it would also be good to know the names of the ltk classes you used when you wrote them. So that I could easily override them if I needed to:

So (ideally) something like:

{"ids": [],
  "classes": ["Item", "paramtitle", "measure", "short", "param", "altlabel", "noborder"],
  "ltkclasses": [ltk-input, ....],
  ".css": True }

@laffra
Copy link
Collaborator

laffra commented Feb 7, 2025 via email

@laffra
Copy link
Collaborator

laffra commented Feb 7, 2025 via email

@Neon22
Copy link
Author

Neon22 commented Feb 7, 2025

OK. Thanks for considering it.
For css I was looking for something like this:

class Widget(object):
    """Base class for LTK widgets."""
    classes = []
    instances = {}
    element = None
    tag = "div"
    used = False   # add this

and in def css(self, prop, value=None):

self.used = True  # indicate user added a manual .css()
if isinstance(prop, dict):
    prop = to_js(prop)
return self.element.css(prop, value) if value is not None else self.element.css(prop)

Then a post process could look to see if it was used...
I will work it out. Thanks for looking into this for me.

@Neon22
Copy link
Author

Neon22 commented Feb 7, 2025

OK. Thanks Chris. looked at 2.23. I can work with that.
Thanks heaps

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

No branches or pull requests

2 participants