ENB: Why I think keyword args are benign

[Edward K. Ream]

Vitalije and I are having a discussion about programming style as part of #862.  Here, I'd like to explain the programming style that I use throughout Leo.  It's possible that this style will change significantly as the result of our discussion, but I doubt it.

Leo has two related commands: paste-node and paste-retaining-clones. Here is the actual code.

@g.commander_command('paste-retaining-clones')
def pasteOutlineRetainingClones(self, event=None):
    '''Paste an outline into the present outline from the clipboard.
    Nodes *retain* their original identify.'''
    c = self
    return c.pasteOutline(reassignIndices=False)

# To cut and paste between apps, just copy into an empty body first, then copy to Leo's clipboard.

@g.commander_command('paste-node')
def pasteOutline(self, event=None,
    reassignIndices=True,
    redrawFlag=True,
    s=None,
    tempOutline=False, # True: don't make entries in the gnxDict.
    undoFlag=True
):
    '''
    Paste an outline into the present outline from the clipboard.
    Nodes do *not* retain their original identify.
    '''
    c = self
    if s is None:
        s = g.app.gui.getTextFromClipboard()
    pasteAsClone = not reassignIndices
    # commenting following block fixes #478
    #if pasteAsClone and g.app.paste_c != c:
    #    g.es('illegal paste-retaining-clones', color='red')
    #    g.es('only valid in same outline.')
    #    return
    undoType = 'Paste Node' if reassignIndices else 'Paste As Clone'
    c.endEditing()
    if not s or not c.canPasteOutline(s):
        return # This should never happen.
    isLeo = g.match(s, 0, g.app.prolog_prefix_string)
    vnodeInfoDict = computeVnodeInfoDict(c) if pasteAsClone else {}
    # create a *position* to be pasted.
    if isLeo:
        pasted = c.fileCommands.getLeoOutlineFromClipboard(s, reassignIndices, tempOutline)
    if not pasted:
        # 2016/10/06:
        # We no longer support pasting MORE outlines. Use import-MORE-files instead.
        return None
    if pasteAsClone:
        copiedBunchList = computeCopiedBunchList(c, pasted, vnodeInfoDict)
    else:
        copiedBunchList = []
    if undoFlag:
        undoData = c.undoer.beforeInsertNode(c.p,
            pasteAsClone=pasteAsClone, copiedBunchList=copiedBunchList)
    c.validateOutline()
    if not tempOutline:
        # Fix #427: Don't check for duplicate vnodes.
        c.checkOutline()
    c.selectPosition(pasted)
    pasted.setDirty()
    c.setChanged(True, redrawFlag=redrawFlag) # Prevent flash when fixing #387.
    # paste as first child if back is expanded.
    back = pasted.back()
    if back and back.hasChildren() and back.isExpanded():
        # 2011/06/21: fixed hanger: test back.hasChildren().
        pasted.moveToNthChildOf(back, 0)
    if pasteAsClone:
        # Set dirty bits for ancestors of *all* pasted nodes.
        # Note: the setDescendentsDirty flag does not do what we want.
        for p in pasted.self_and_subtree():
            p.setAllAncestorAtFileNodesDirty(
                setDescendentsDirty=False)
    if undoFlag:
        c.undoer.afterInsertNode(pasted, undoType, undoData)
    if redrawFlag:
        c.redraw(pasted)
        c.recolor()
    return pasted

This isn't the easiest code to understand, but imo the details don't matter.  What does matter is that:

1. Each command handles everything about the command, including undo and redraw.

2. The meaning of each kwarg is clear:

- reassignIndices:  distinguishes between paste-node and paste-retaining clones.
- redrawFlag: suppresses calls to c.redraw when False.
- s:  The to-be-pasted text.  Get the text from the clipboard if None.
- tempOutline: don't make entries in the gnxDict.
- undoFlag: Handle undo unless False.

I admit, the tempOutline flag is a bit mysterious.  In fact, it can be removed.  See below.

cff shows only three calls to c.pasteOutline in Leo's core:

1. From c_oc.pasteOutlineRetainingClones (shown above)

    return c.pasteOutline(reassignIndices=False)

2. From abbrev.paste_tree:

    p = c.pasteOutline(s=s, redrawFlag=False, undoFlag=False)

3. From LM.openEmptyWorkBook:

    p = c.pasteOutline()

Note: unit tests call only c.pasteOutline() and c.pasteOutlineRetainingClones().

From my point of view, the only important question is: are these calls understandable in context?

And clearly, the answer is, unequivocally yes. Indeed, the only "difficult" call is from abbrev.paste_tree, but in context it is obvious that the to-be-pasted text comes from the abbreviation node, not the clipboard.

Removing kwargs

Vitalije's suggested alternative, if I understand him fully, is to remove all kwargs!  The post script shows the refactored c_oc.pasteOutline and c_oc.pasteOutlineRetainingClones, and a third method which would also be needed. Imo, this refactoring would be catastrophic, for the following reasons:

1. There is extreme duplication of code, to no purpose whatsoever.

2. There is no easy way to compare and contrast the similarities and differences between the two commands.

3. The only way to support abbrev.paste_tree would be to create a third method, c.pasteOutlineFromString, also shown in the post script.  Indeed, all minor variants of code require completely new versions of code, duplicating all common code, no matter how complex.

Summary

The coding pattern just discussed forms the basis for all of Leo's commands.

kwargs are intended to be a way of making small, easily understood modifications to existing code.

I see no reason to enforce a faux-functional programming style on all of Leo's code base. To the contrary, kwargs reduce code duplication and explicitly show the differences between minor variants of code.

Unnecessary kwargs should be removed when possible.  It looks like the tempOutline kwarg could be removed. That might break existing scripts, but that's unlikely.  Removing this kwarg could have the happy side effect of simplifying fc.getLeoOutlineFromClipboard.

Edward

P. S. Here are the refactored (untested) versions of c_oc.pasteOutline, c_oc.pasteOutlineRetainingClones, and the newly-necessary c.pasteOutlineFromString. Imo, this kind of code duplication should be avoided at all costs:

@g.commander_command('paste-node')
def pasteOutline(self, event=None):
    '''
    Paste an outline into the present outline from the clipboard.
    Nodes do *not* retain their original identify.
    '''
    c = self
    s = g.app.gui.getTextFromClipboard()
    undoType = 'Paste Node'
    c.endEditing()
    if not c.canPasteOutline(s):
        return # This should never happen.
    isLeo = g.match(s, 0, g.app.prolog_prefix_string)
    vnodeInfoDict = {}
    # create a *position* to be pasted.
    if isLeo:
        pasted = c.fileCommands.getLeoOutlineFromClipboard(s, reassignIndices)
    if not pasted:
        # We no longer support pasting MORE outlines. Use import-MORE-files instead.
        return None
    copiedBunchList = []
    undoData = c.undoer.beforeInsertNode(c.p,
        pasteAsClone=False, copiedBunchList=copiedBunchList)
    c.validateOutline()
    c.checkOutline()
    c.selectPosition(pasted)
    pasted.setDirty()
    c.setChanged(True)
    # paste as first child if back is expanded.
    back = pasted.back()
    if back and back.hasChildren() and back.isExpanded():
        # 2011/06/21: fixed hanger: test back.hasChildren().
        pasted.moveToNthChildOf(back, 0)
    c.undoer.afterInsertNode(pasted, undoType, undoData)
    c.redraw(pasted)
    c.recolor()
    return pasted

@g.commander_command('paste-retaining-clones')
def pasteOutlineRetainingClones(self, event=None):
    '''
    Paste an outline into the present outline from the clipboard.
    Nodes do *not* retain their original identify.
    '''
    c = self
    s = g.app.gui.getTextFromClipboard()
    undoType = 'Paste As Clone'
    c.endEditing()
    if not s or not c.canPasteOutline(s):
        return # This should never happen.
    isLeo = g.match(s, 0, g.app.prolog_prefix_string)
    vnodeInfoDict = computeVnodeInfoDict(c)
    # create a *position* to be pasted.
    if isLeo:
        pasted = c.fileCommands.getLeoOutlineFromClipboard(s, reassignIndices)
    if not pasted:
        # 2016/10/06:
        # We no longer support pasting MORE outlines. Use import-MORE-files instead.
        return None
    copiedBunchList = computeCopiedBunchList(c, pasted, vnodeInfoDict)
    undoData = c.undoer.beforeInsertNode(c.p,
        pasteAsClone=True, copiedBunchList=copiedBunchList)
    c.validateOutline()
    c.checkOutline()
    c.selectPosition(pasted)
    pasted.setDirty()
    c.setChanged(True, redrawFlag=redrawFlag) # Prevent flash when fixing #387.
    # paste as first child if back is expanded.
    back = pasted.back()
    if back and back.hasChildren() and back.isExpanded():
        # 2011/06/21: fixed hanger: test back.hasChildren().
        pasted.moveToNthChildOf(back, 0)
    # Set dirty bits for ancestors of *all* pasted nodes.
    # Note: the setDescendentsDirty flag does not do what we want.
    for p in pasted.self_and_subtree():
        p.setAllAncestorAtFileNodesDirty(
            setDescendentsDirty=False)
    c.undoer.afterInsertNode(pasted, undoType, undoData)
    c.redraw(pasted)
    c.recolor()
    return pasted

def pasteOutlineFromString(self, s):
    '''
    Paste an outline into the present outline from the clipboard.
    Nodes do *not* retain their original identify.
    '''
    c = self
    c.endEditing()
    if not c.canPasteOutline(s):
        return
    isLeo = g.match(s, 0, g.app.prolog_prefix_string)
    vnodeInfoDict = {}
    # create a *position* to be pasted.
    if isLeo:
        pasted = c.fileCommands.getLeoOutlineFromClipboard(s)
    if not pasted:
        # We no longer support pasting MORE outlines. Use import-MORE-files instead.
        return None
    c.validateOutline()
    c.checkOutline()
    c.selectPosition(pasted)
    pasted.setDirty()
    c.setChanged(True, redrawFlag=False)
    # paste as first child if back is expanded.
    back = pasted.back()
    if back and back.hasChildren() and back.isExpanded():
        # 2011/06/21: fixed hanger: test back.hasChildren().
        pasted.moveToNthChildOf(back, 0)
    return pasted

EKR

[Edward K. Ream]

On Monday, April 23, 2018 at 3:58:08 AM UTC-5, Edward K. Ream wrote:

> I admit, the tempOutline flag is a bit mysterious.

Indeed it is.  It was added to fix #427, but I can find no calls with tempOutline set.

Edward

[vitalije]

    s = g.app.gui.getTextFromClipboard()
    undoType = 'Paste As Clone'
    c.endEditing()
    if not s or not c.canPasteOutline(s):
        return # This should never happen.
    isLeo = g.match(s, 0, g.app.prolog_prefix_string)
    vnodeInfoDict = computeVnodeInfoDict(c)
    # create a *position* to be pasted.
    if isLeo:
        pasted = c.fileCommands.getLeoOutlineFromClipboard(s, reassignIndices)

This part of code is present in almost all three kinds of paste functions as you have refactored them. However, if you look into c.canPasteOutline you will see that it repeats almost all of the above. It checks to see if s is None and if so it takes value from clipboard, then it checks if it starts with g.app_prolog_prefix_string and if everything is checked returns True. In order to avoid code duplication some other duplication have been made. And it is not easy to spot the duplication and unnecessary code unless you really dive into this code.

Proposing three different methods for three kinds of paste commands, I haven't imagined them to be like those. This is just first layer of decomposing code. For example in all three implementations you are using c.fileCommands.getOutlineFromClipboard passing kw flag again. And also same switch is passed to undo methods. Instead, I would recommend further decomposition down the road. 

By the way, getOutlineFromClipboard ends with call to c.selectPosition(p) where p is newly pasted node. In the above implementations there is also call to select the same position. 

Also, getOutlineFromClipboard creates at first v node, and then creates a position to hold this v node, then in the above implementations, calls methods of the position class moveToNthChildOf or linkAsNthChildOf which are delegating their jobs to p.v methods. It would be much simpler if getOutlineFromClipboard returns v instance, and then just make link with parent_v. 

A variable named undoType is introduced in first two methods, just to be used once in the end. It would suffice to use constant string instead. This variable is necessary if pasteOutline performs both kinds of operation. By separating functions this variable can be eliminated and code complexity further reduced. 

For undoing paste-retaining-clones it would be enough to just cut the link between parent_v and the pasted v node. By definition all nodes inside the pasted outline are exactly the same after paste command. The only difference is one more parent element in v.parents and just in top level pasted node. That is why I strongly suspect that computing a copied bunch is unnecessary too.

copiedBunchList = computeCopiedBunchList(c, pasted, vnodeInfoDict)
undoData = c.undoer.beforeInsertNode(c.p,
                pasteAsClone=True, copiedBunchList=copiedBunchList)
    

And there again is a method c.undoer.beforeInsertNode which needs kw flags pasteAsClone to be passed. I would recommend also to have c.undoer.beforePasteClone and c.undoer.afterPasteClone. which should be extremely simply. 

After all if there are common parts of these three kinds of paste methods then we can extract them and have:

def pasteOutlineRetainingClones(self, p):

    (childIndex, undodata, txt,...) = prePasteNodeCommon(p,

                                        c.undoer.beforePasteRetainingClone)

    ...

    # specific to retaining clones variant

    v = getVnodeFromString(txt) # somehow see our discussion on #862

    if v:

        postPasteNodeCommon(p, v, undodata, c.undoer.afterPasteRetainingClone)

def pasteOutline(self, p):

    (childIndex, undodata, txt, ...) = prePasteNodeCommon(p,

                                        c.undoer.beforePasteNoClone)

    ...

    # specific to paste no clones

    v = getVnodeFromStringWithNewIndices(txt)

    if v:

        postPasteNodeCommon(p, v, c.undoer.afterPasteNoClone)

def pasteFromString(self, p, txt):

    (childIndex, undodata, dummy, ...) = prePasteNodeCommon(p,

                                        lambda *args: None,

                                        txt)

    ...

    # specific to paste from string

    v = getVnodeFromStringWithNewIndices(txt)

    # no undo

def prePasteNodeCommon(p, undoMethod):

    c.endEditing()

    ...

    # decide where to paste: parent_v and childIndex
    # based on expanded/collapsed state of current position

    ...

    txt = g.app.gui.getTextFromClipboard()

    undodata = undoMethod(c.undoer, ...)

    return childIndex, undodata, txt, ...

def postPasteNodeCommon(p, v, undodata, undoMethod):

    # make link

    v.parents.append(p.v)

    p.v.children.insert(childIndex, v)

    undoMethod(c.undoer, undodata)

This is not complete code just a blueprint. But implemented this way would look much better and point out in more contrast the difference from the current implementation.

Vitalije

[Edward K. Ream]

On Mon, Apr 23, 2018 at 6:50 AM, vitalije <vita...@gmail.com> wrote:

​> ​

In order to avoid code duplication some other duplication have been made. And it is not easy to spot the duplication and unnecessary code unless you really dive into this code.

​...​

​> ​

Proposing three different methods for three kinds of paste commands, I haven't imagined them to be like those. This is just first layer of decomposing code.

​...​

​> ​

This is not complete code just a blueprint. But implemented this way would look much better and point out in more contrast the difference from the current implementation.

​All interesting comments.  I would like to see the complete new code in a new branch.

> fc.getOutlineFromClipboard creates at first v node, and then creates a position to hold this v node, then in the above implementations, calls methods of the position class moveToNthChildOf or linkAsNthChildOf which are delegating their jobs to p.v methods. It would be much simpler if getOutlineFromClipboard returns v instance, and then just make link with parent_v. 

Maybe, but this would make fc.getOutlineFromClipboard less self contained. This matters, because it can be called from several places, including the read logic. 

I am now going to fix #862: paste-retaining-clones can corrupt the outline The fix looks like it will be in fc.getLeoOutlineFromClipboard, or perhaps its helpers.  I doubt whether any refactoring would make the bug, or its fix, easier to spot.

Edward

[Edward K. Ream]

On Monday, April 23, 2018 at 3:58:08 AM UTC-5, Edward K. Ream wrote:

I admit, the tempOutline flag is a bit mysterious.

Searching leo-editor for "tempOutline" yields this discussion, in 2015.  This may have predated the clone-find commands.

The paste-retaining-clones branch contains work on #862. The first thing I did in the branch was to remove the tempOutline kwarg.

Edward

[vitalije]

Maybe, but this would make fc.getOutlineFromClipboard less self contained. This matters, because it can be called from several places, including the read logic. 

I can't see why that would be the case? Can you explain why would getOutlineFromClipboard become less self contained if it returns vnode instance instead of position? 

As a matter of fact I remember that I have replaced one particular call to this method with the call to another getVnodeFromClipboard especially to prevent it from calling c.selectPosition during the initialization before tree becomes ready. It is fully self-contained and can be called from anywhere and doesn't affect c state in any way. 

[Edward K. Ream]

On Mon, Apr 23, 2018 at 10:49 AM, vitalije <vita...@gmail.com> wrote:

Can you explain why would

​​

getOutlineFromClipboard become less self contained if it returns vnode instance instead of position? 

​Positions are more useful. p.v is a position's vnode, but in general there is no way to go from a vnode v to a unique position.​

 

​More importantly, we don't want to force callers to duplicate the code in ​

​

getOutlineFromClipboard, especially the code involving fc.gnxDict.

​Edward

[vitalije]

​All interesting comments.  I would like to see the complete new code in a new branch. 

ATM I can't update my Leo and create new branch but here is a complete code in experimental outline. I hope google will accept Leo document as attachment to this message.

Whole code has 150LOC, which is a bit more than I expected, but it contains everything. Perhaps you can do better job regarding the undo/redo functionality. I can't dive into undoer code right now so I have re-implemented everything from scratch. The script uses leoUndo just to pushBead with the information and undo/redo helpers.

Vitalije

PS: I have just manually tested copying, pasting with/or without retaining clones and undoing/redoing. 

[vitalije]

The pasteFromString variant is missing, but it would be trivial to add. Just skip everything undo/redo related. Basically if the call to c.undoer.pushBead is commented out, and txt accepted as input argument instead of retrieving it from clipboard, you get pasteFromString.

Vitalije

[Edward K. Ream]

On Mon, Apr 23, 2018 at 1:46 PM, vitalije <vita...@gmail.com> wrote:

​>

​

here is a complete code in experimental outline. ​

​Superb code.  Many thanks for it.​

 

​> ​

Perhaps you can do better job regarding the undo/redo functionality. I can't dive into undoer code right now so I have re-implemented everything from scratch.

​Perhaps Leo's undo code could be rewritten along the same lines.  Many years ago the late Bernhard Mulder proposed simplifying the undo code, but I didn't understand how it could be done.

​

​> ​

PS: I have just manually tested copying, pasting with/or without retaining clones and undoing/redoing. 

​It's a great prototype.  I'll study it carefully.

> The pasteFromString variant is missing, but it would be trivial to add. Just skip everything undo/redo related.

In general, we don't want to force a major refactoring of code just because some client code wants to tweak existing core code.  In this case, however, you have made a strong case for re-imagining everything.

We will also need a paste-as-xml command, so that people can paste the xml-version of an outline into an external editor.  But there would be no need to read copied xml back into Leo itself.

Assuming the prototype can be integrated into Leo, we could consider using json as an alternative format for .leo files.  The existing sax-based read code must remain indefinitely, to support Leo's xml-based file format, but the json format might be preferable.

Terry, do you have any comments?

Edward

[rengel]

On Monday, April 23, 2018 at 10:58:08 AM UTC+2, Edward K. Ream wrote:

... a discussion about programming style ...

...

   2. The meaning of each kwarg is clear:

   - reassignIndices:  distinguishes between paste-node and paste-retaining clones.
   - redrawFlag: suppresses calls to c.redraw when False.
   - s:  The to-be-pasted text.  Get the text from the clipboard if None.
   - tempOutline: don't make entries in the gnxDict.
   - undoFlag: Handle undo unless False.

That made me wonder: Do you plan sometime in the future to make the code PEP8-compliant? Does Leo have any kind of 'style guide' or naming conventions?

Just a cursory glance at the parameters shows that at least some explanations/comments could be removed if other names were used, i.d. if the names wouldn't express what a paramer is (i.e. a 'flag'), but what action it stands for. Examples:

Instead of                                                                                                                         one could use

redrawFlag                                                      redrawOutline
undoFlag                                                        enableUndo
tempOutline=False, # True: don't make entries in the gnxDict.   updateGnxDict


[Edward K. Ream]

On Tue, Apr 24, 2018 at 6:17 AM, rengel <reinhard...@gmail.com> wrote:

That made me wonder: Do you plan sometime in the future to make the code PEP8-compliant?

​Probably not in the sense you might be thinking of.
​

 

Does Leo have any kind of 'style guide' or naming conventions?

​New classes should follow the suggested naming conventions.  Existing classes won't change in this regard.

Just a cursory glance at the parameters shows that at least some explanations/comments could be removed if other names were used, i.d. if the names wouldn't express what a parameter is (i.e. a 'flag'), but what action it stands for.

​The "flag" suffix is typically used when there is a method that whose name would otherwise collided with the var or ivar.

Edward

[Terry Brown]

On April 24, 2018 4:58:52 AM CDT, "Edward K. Ream" <edre...@gmail.com> wrote:
>On Mon, Apr 23, 2018 at 1:46 PM, vitalije <vita...@gmail.com> wrote:
>
>Terry, do you have any comments?

I haven't been following the code, just the discussion. I'm in favor of incremental simplification where ever practical. Remember json's limitations, all keys are strings, and only int / float / list / dict / string types stored simply.

Cheers - Terry

--
Sent from my Android device with K-9 Mail. Please excuse my brevity.

[Edward K. Ream]

On Tue, Apr 24, 2018 at 7:11 AM, Terry Brown <terry...@gmail.com> wrote:

I haven't been following the code, just the discussion. I'm in favor of incremental simplification where ever practical.

​I am especially interested in your opinion about supporting json as an alternative format for .leo files.​

 
 

Remember json's limitations, all keys are strings, and only int / float / list / dict / string types stored simply.

​Ouch.  At present, the code is:

def copyOutline(p):
    g.app.gui.replaceClipboardWith(json.dumps(p2list(p)))

def p2list(p):
    return [p.h, p.b, p.gnx, p.v.statusBits, p.v.unknownAttributes,
             [p2list(p2) for p2 in p.children()]]

It looks like json.dumps may have problems with p.v.unknownAttributes fields.

Again, not a gotcha, but something must be done.

Edward

[Terry Brown]

On Tue, 24 Apr 2018 07:38:35 -0500

"Edward K. Ream" <edre...@gmail.com> wrote:

> It looks like json.dumps may have problems with p.v.unknownAttributes
> fields.

In theory yes, in practice not so much, although still a potential
problem.

The leo_cloud plugin stores outlines (well, subtrees, without expansion
info. etc.) in JSON. The two issues I found were todo.py storing
datetime dates, and the tags plugin storing sets.

Python's JSON module supports converting types on write - easy to make
set a list, and I think I converted dates to YYYYMMDDTHHMMSS format.

Then you have to handle them on read - so I tweaked tags to convert to
set, and todo was already reading text dates, seeing the datetime.dates
had been a problem with different pickle versions.

So I guess in general I didn't find actual problems, but there's still
the issue that currently someone can do:

p.v.u['pantry'] = {
('beans', 'aug'): 17,
('beans', 'jun'): 12,
('flour', 'jul'): 11,
('flour', 'aug'): 10,
}

and that just doesn't work in JSON.

I don't think we want to go down the track of letting users register
JSON read/write helpers for certain keys in p.v.u. *Maybe* we could
handle the case of ints/floats/tuples/(frozen sets) as keys by
converting to/from string representations.

Cheers -Terry

[Edward K. Ream]

On Tue, Apr 24, 2018 at 8:34 AM, Terry Brown <terry...@gmail.com> wrote:

On Tue, 24 Apr 2018 07:38:35 -0500
"Edward K. Ream" <edre...@gmail.com> wrote:

​​

> It looks like json.dumps may have problems with p.v.unknownAttributes
> fields.

In theory yes, in practice not so much, although still a potential

​ ​

problem.

​Thanks for these comments.  ​

 

​I've created the info issue #872 ​to remember them.

Edward

[Matt Wilkie]

We will also need a paste-as-xml command, so that people can paste the xml-version of an outline into an external editor.  But there would be no

need to read copied xml back into Leo itself.

No, at least not for that reason. "Leo >> Copy node" ... "Notepad >> Paste" already pastes XML (on Windows. Other OSes differ perhaps?)

Matt

[David Szent-Györgyi]

On Tuesday, April 24, 2018 at 8:38:37 AM UTC-4, Edward K. Ream wrote:

It looks like json.dumps may have problems with p.v.unknownAttributes fields.

Again, not a gotcha, but something must be done.

Have you had time to look into AXON as a better-for-the-purpose alternative to JSON?  

[Terry Brown]

> <https://groups.google.com/d/msg/leo-editor/0oXX21fFYNs/OdBbgFlMAAAJ>?

Yes, thanks for the pointer. It doesn't really pop up as having huge
buy in in searches. And I'm not sure it makes a huge difference, native
datetime support and YAML formatting, and sets and sorted dicts I
guess... but still not tuples as keys and arbitrary types as values.
So I think if we do switch from pickle we can just specify uA's need to
be JSON serialisable.

Cheers -Terry

[Edward K. Ream]

On Tue, May 1, 2018 at 2:34 PM, Terry Brown <terry...@gmail.com> wrote:

So I think if we do switch from pickle we can just specify uA's need to
be JSON

​

serializable.

​Sounds reasonable.  I wonder, is there any plausible situation in which a ​uA wouldn't be json serializable?

Anyone have thoughts about this?

Edward

[Terry Brown]

On Tue, 1 May 2018 16:05:18 -0500

"Edward K. Ream" <edre...@gmail.com> wrote:

> ​Sounds reasonable. I wonder, is there any plausible situation in
> which a ​uA wouldn't be json serializable?
>
> Anyone have thoughts about this?

Well, every time a user's placed a non-JSON-serializable key or value
in uA.

import datetime
import json
json.dumps({'a': 1}) # valid
json.dumps({1: 1}) # works, but changes type of key
json.dumps({1.2: 1}) # works, but changes type of key
json.dumps({(1,1): 1}) # TypeError, never happens?
json.dumps({'a': datetime.datetime(2015, 1, 1, 3)}) # TypeError
json.dumps({'a': set((1, 2))}) # TypeError

I'll try and scan my .leo files and see how many real world problems
there are.

Cheers -Terry

[Edward K. Ream]

On Tue, May 1, 2018 at 4:16 PM, Terry Brown <terry...@gmail.com> wrote:

I'll try and scan my .leo files and see how many real world problems
there are.

​Ok.  Thanks.

I think we can require json keys retroactively unless somebody strongly objects.

Edward