ncssar / sartopo_python

Python calls for the caltopo / sartopo API
GNU General Public License v3.0
14 stars 2 forks source link

'title' value is blank for all assignment features in CTD 4221 #37

Closed caver456 closed 1 year ago

caver456 commented 2 years ago

Apparently in CTD 4221, assignment 'title' is always an empty string regardless of letter and number. Bug report is being filed.

Beyond that, various code in sartopo_python fails with KeyError: 'title' - sure enough, the cached feature has no key named 'title', which is strange - how and why is the title key-value-pair getting removed?

Rather than play whack-a-mole in parts of code that refer to ['properties']['title'], try and deal with it earlier in the flow, when the cache entry is created. Everything stems from the get /since/ response, so that's probably the right place to do it. The formula is simple: title = letter+space+number

This is in the gray area between bug and enhancement. Calling it a bug since sartopo_python should deal with whatever CTD 4221 dishes up.

caver456 commented 2 years ago

This was addressed in code several commits ago; I've been testing with 4214 which doesn't have the issue, so, we know the code fix doesn't break 4214, but it's not clear if there are memory leaks:

# since CTD 4221 returns 'title' as an empty string for all assignments,
#  set 'title' to <letter><space><number> for all assignments here
# this code looks fairly resource intensive; for a map with 50 assignments, initial sync
#  is about 6.5% slower with this if clause than without, but it would be good to profile
#  memory consumption too - is this calling .keys() and creating new lists each time?
#  maybe better to wrap it all in try/except, but, would that iterate over all features?
if 'result' in rj.keys() and 'state' in rj['result'].keys() and 'features' in rj['result']['state'].keys():
    alist=[f for f in rj['result']['state']['features'] if 'properties' in f.keys() and 'class' in f['properties'].keys() and f['properties']['class'].lower()=='assignment']
    for a in alist:
        a['properties']['title']=a['properties']['letter']+' '+a['properties']['number']
caver456 commented 1 year ago

Did some more time profiling, though didn't try to do any memory profiling which would probably be a long-term test. Maybe best to insert some memory logging into the main code, so that memory consumption at various points during actual usage is saved in the transcript, for analysis later.

Time profiling didn't show anything conclusive. Sometimes, the existing key-check-and-comprehension method is a bit (5% or 20msec) faster that the try-except method; sometimes it is the other way around. In both cases, the entire clause, looping all over features in an 18MB map, took 1msec.

So: nothing to do here for now. Closing this issue. Could be reopened later if long-term memory profiling shows a memory leak.

Here's the try-except clause (with a few things hardcoded for testing):

                        try:
                            rjsf=rj['result']['state']['features']
                        except:
                            logging.info('exception1')
                            pass
                        else:
                            # logging.info('t0')
                            for f in rjsf:
                                # logging.info('t1')
                                try:
                                    # logging.info(' t2')
                                    p=f['properties']
                                    if p['class'].lower()=='assignment' and p['title']=='CB':
                                        logging.info('  found')
                                        letter=p.get('letter','')
                                        number=p.get('number','')
                                        joiner=''
                                        if letter and number:
                                            joiner=' -- ' # two dashes to test
                                        p['title']=str(letter)+joiner+str(number)
                                        logging.info('  changed title to "'+p['title']+'"')
                                    # logging.info(' t3')
                                except:
                                    logging.info('exception2')
                                    pass