[17:47:39] hi [17:51:23] hello [17:52:18] did you manage to do the clean-up and code-comment the genericglam? [17:53:25] Oh I was not focussing on genericGLAM [17:54:39] which part do want more comments? the two boolean functions? [17:54:50] https://github.com/infobliss/sibutest2/blob/master/libraries/GenericGLAM.py#L16? [17:54:56] yes [17:55:09] ok give me 10 minutes [17:55:15] please listen to the comments I and zhuyifei give [17:55:27] I've given this improvement now 3 or 4 times [17:55:33] just explain what the return value means [17:55:33] these simple things just do them [17:56:07] Actually I have never used them [17:56:14] these two methods [17:56:37] you don't have to [17:56:44] Maybe I'm wrong and zhuyifei1999_ can say so. But the idea is that you will call these functions for each glam [17:56:56] you haven't implemented every single glam yet [17:57:00] having them here provides a template which defaults to false if it isn't implemented for a glam [17:57:08] so you don't know if they will be used or now [17:57:20] exactly [17:57:37] ok please give me 10 minutes [18:03:22] ok [18:09:36] done [18:09:42] Please check now [18:13:08] https://github.com/infobliss/sibutest2/blob/master/glams/NationaalArchiefGLAM.py#L94 you can return the license for the correct cases and use that function to determine the license [18:13:14] unless that is considered bad coding practice [18:13:48] that was indeed the case in the earlier version [18:14:19] yes but it wasn't returning false for the incorrect case [18:14:30] https://github.com/infobliss/sibutest2/blob/master/libraries/GenericGLAM.py#L20 is connected to that, for the rest good [18:14:40] ok [18:14:47] https://github.com/infobliss/sibutest2/blob/master/libraries/GenericGLAM.py#L29 uuid? [18:15:03] why doesn't it just return 1 thumbnail? [18:15:20] you did all the fancy code in the nationaal archief code for the thumbing and delivering a page I believe [18:15:34] that means that for every glam everybody always has to do that complicated fancy code [18:15:48] while it should be quite doable to make it generic [18:16:19] if the glam code provides a thumbnail combining the thumbnails into lists should be done somewhere else [18:17:14] imo thumbnail_locator() is specifically used for multiple image upload [18:17:16] is https://github.com/infobliss/sibutest2/blob/master/libraries/GenericGLAM.py#L25 implemented in https://github.com/infobliss/sibutest2/blob/master/glams/NationaalArchiefGLAM.py#L43 [18:17:22] I honestly have no idea what get_thumb_url is doing [18:17:39] yes that was my next point, it's very complicated unreadable uncommented code [18:18:23] ok 5 more minutes please [18:18:31] why did you use xml infobliss ? [18:18:40] wasn't this available as json? [18:19:06] please add some example url in the comments of that function as well [18:19:29] example url: https://github.com/infobliss/sibutest2/blob/master/glams/NationaalArchiefGLAM.py#L77 [18:19:58] searchString is provided by user in the html form [18:20:30] what happens when there is a space in the searchstring? [18:20:41] %20 [18:20:50] "leiden markt" [18:20:56] %20 is evil [18:21:06] ok [18:21:35] https://github.com/infobliss/sibutest2/blob/master/glams/NationaalArchiefGLAM.py#L77 where's urlescape? [18:22:27] + is what you should replace the spaces with [18:22:35] in the NA case [18:23:18] oh I didn't know that [18:23:20] yeah, urlescape usually replace space with + [18:23:34] eg. search google [18:23:36] anyhow, the search_string shouldn't be in a function called gallery builder [18:23:56] what you want is a function which you give a searchstring and returns a list of identifiers [18:24:04] then those identifiers you can get the info for with the standard code [18:24:16] (using a for loop outside of the NA code [18:24:41] but before that you call the license checker function [18:24:45] for each of those files [18:24:58] those which do not return false you get the thumbnail from [18:25:31] for the thumbnail function: you send it an identifier [18:25:43] and optionally you send it the pixel sizes [18:25:55] and it returns a link to a thumbnail [18:26:14] this is a more logical flow [18:26:36] currently you are doing a lot of the things which are generic in the NA-specific code [18:26:50] in the NA-specific code there should be the following functions [18:27:02] searchstring -> list of identifiers [18:27:12] identifier -> license OK? [18:27:20] identifier -> thumbnail [18:28:09] identifier + categories -> wikitext/infobox template, image_url, title, (optionally categories depending on where you construct the wikitext) [18:28:32] than you can build generic code which can do all the communication between those functions [18:29:07] and which transfers a list of thumbnails + a list of identifiers (from the for loops) into a page [18:30:22] do you follow infobliss? [18:30:32] and zhuyifei1999_ do you agree with such a flow? [18:30:35] yeah [18:31:14] you agree also infobliss and know what to change to get it to work like that? [18:32:10] let me give it a thought [18:34:20] to connect the other glam (AM) I need to know what these functions will look like who call the AM code [18:34:32] and preferably we do as much generic as possible [18:42:35] sorry my net was disconnected [18:43:11] so I gave it a thought [18:43:20] I have one doubt though [18:44:00] what I understand is that for being able to build the image gallery for multiple upload we need the following 2 things [18:44:29] uuid_list and thumb_url_list [18:44:42] which are both glam specific [18:45:11] so which part of extracting these two from the searchString can be made generic? [18:46:27] uuid is a national archive specific name [18:46:42] can you please start calling it identifier [18:46:51] it is confusing [18:47:06] can we call it uid? [18:47:11] why? [18:47:20] short for unique identifier [18:47:20] image_identifier [18:47:33] or image_id [18:47:46] although it's not always the image id [18:47:57] uid usually means user id [18:48:09] either image_id, id or identifier [18:48:19] uuid is almost always specifically UUID [18:48:20] and there's no need to pick short over informing [18:48:43] to get back to the question [18:49:00] you get an identifier list from the [20:27] searchstring -> list of identifiers [18:49:07] function [18:49:44] you get a list of thumb urls by looping over the identifier list and then doing the following 2 functuions: [20:27] identifier -> license OK? [20:27] identifier -> thumbnail [18:50:04] the loop is what can be made generic [18:50:23] and the checking for a license and then calling the function for the thumb [18:50:45] that doesn't mean they should go in the genericGlam.py code [18:51:14] they should go in the code where you call the different glams etc. the main code basicly [18:51:29] app.py? [18:51:59] not sure if it is good to stick it there or make a new file which gets called by app.py [18:52:11] app.py is front-end to back-end communication [18:52:22] this is back-end stuff [18:53:22] so preferably shouldn't be done in the communication code [18:53:59] I think that is https://en.wikipedia.org/wiki/Model%E2%80%93view%E2%80%93controller [18:54:09] zhuyifei1999_? [18:54:30] meh [18:54:46] separation of front-end, communication and back-end is that MVC? or a bit more generic [18:55:18] I'm not 100% familiar with all the terms of mvc [18:55:32] ok well let's keep that out of the discussion then [18:55:45] I've learned it in the past but not 100% sure it's applicable [18:56:06] but I think the separation of those parts (front, communication, back) is important for logical code [18:56:14] yes [18:56:33] which means the amount of things done by app.py should be reduced [18:56:38] it's the communication code [18:56:51] basvb: "you get a list of thumb urls by looping over the identifier list" : how can this be made generic? [18:57:27] pesudocode: Image(id).get_thumb() [18:57:44] identifier_list = glam.identifiers_from_searchstring(searchstring) [18:57:51] for id in identifier list: [18:58:08] if glam.license_ok(id): [18:58:21] glam.get_thumb(id) [18:58:43] can we not make glam singletons? [18:59:10] you could glam(id).license_ok [19:00:21] otherwise a lot of the exposed methods want the id [19:00:40] so basicly make it a class with the id as a self parameter? [19:00:47] I'm a bit out of dept here again ;) [19:01:10] no, it's like what I asked ages ago: what should each instance represent [19:01:26] if each instance represent a glam they are singletons [19:01:30] one file upload [19:01:40] as each glam is represented by the class aleady [19:02:23] and for those that could have been instance methods, with singletons they would want the id [19:03:08] till now for each image upload we instantiate an object of the corresponding GLAM class type [19:03:08] I think I'd need an example of the good and the bad way to understand the issue [19:03:11] and there would be no way to store a state (eg cached json/xml) for each image [19:04:11] [21:03] till now for each image upload we instantiate an object of the corresponding GLAM class type -> if you do multi-uploads I was planning on instantiating one times the glam class and calling it with different ids [19:04:23] but i understand that's not the correct way [19:05:00] zhuyifei1999_ had this suggestion to instantiate once per upload iirc [19:05:20] yeah, imagine if you run a bot and Commons().save('COM:Sandbox', 'testedit', summary='lalala') [19:05:55] what does this mean? [19:06:09] why the () before the dot? is commons() a function there? [19:06:22] assume it is a singleton class [19:06:38] or the java style Commons.get_instance() [19:06:45] yes so you'd say: commonsuploader = Commons() [19:06:56] sure yeah [19:06:58] commonsuploader.save(..) [19:07:09] and then 1000 times [19:07:16] exactly [19:07:17] I'd do that normally [19:07:32] * zhuyifei1999_ facepalm [19:07:40] lol [19:07:45] you know there's a class called page right? [19:08:18] I wasn't talking about the pywikibot/wikispecific functions [19:08:29] just how I'd use a class and it's functions [19:08:32] do you seriously text = commons.get(pagename); ...; commons.save(pagename, text) [19:08:37] well, the same applies [19:09:00] don't singleton and have the pagename passed everywhere [19:09:15] I mean everywhere you call a method of the class [19:09:39] i.e. don't pass the image_id everywhere [19:09:55] as in the case if glams are singletons [19:10:27] ok [19:10:38] so initiate it like glam(id) [19:10:43] and then call functions [19:11:02] is that what the issue is? [19:11:22] if you find yourself repeating image_id all the time, you know you can seperate into classmethods and instancemethods, with classmethods continue to behave like singletons, but instance methos have the id [19:11:27] yeah [19:11:36] ok, I do that also sometimes when writing code [19:11:41] didn't get it was about that [19:12:15] search for example should be a classimage [19:12:19] *classmethod [19:12:42] as it does not act on a particulr image, but the glam [19:12:42] which doesn't use the specified ID? [19:12:48] yeah [19:12:59] but license checks should be instace [19:13:03] *instance [19:13:10] as it acts on a single image [19:13:15] so within the nationaalarchief.py there should be a class [19:13:26] image for example [19:13:31] which is called with an ID [19:13:39] constructed with an ID [19:13:52] and has all the ID-related functions for that one image [19:13:59] you can make nationaalarchief that class [19:14:10] you don't have to make another image class [19:14:12] outside of that class is the searchstring to ids function? [19:14:22] true I was just picking a name for the example [19:14:28] a sec [19:14:52] https://docs.python.org/3/library/functions.html#classmethod [19:15:33] so no not outside the class, but inside [19:15:43] ok [19:15:47] mark it with @classmethod decorator [19:16:01] so the self is now cls [19:16:34] or you could do staticmethod which does not give an implicit first argument [19:16:53] learning a lot here [19:17:21] basically no need to instantiate for classmethods, is that correct? [19:17:42] anyways: rule #1: don't repeat yourself [19:17:57] the same applies for image_id in every single method [19:18:01] infobliss_: yes [19:18:05] yep [19:18:52] and this loop over the images can best be done outside of either app.py and outside genericglam.py (which is a template) in a new file? [19:18:56] so you can even do something like MationaalArchief.from_url(url) and let it call the constructor [19:19:26] this loop is still a mystery [19:19:50] what's the loop for exactly? [19:19:57] how is the loop a mistery? [19:20:02] I think my hort term memory just failed [19:20:03] I wrote the whole loop down [19:20:03] suppose user chose NA glam [19:20:10] *short term [19:20:18] yeah [19:20:20] gives identifier : a9946ea0-d0b4-102d-bcf8-003048976d84 [19:20:29] yeah? [19:20:34] you want to get the thumb URL for it [19:20:42] [20:27] and [20:57] [19:21:01] whic is "http://afbeeldingen.gahetna.nl/naa/thumb/1280x1280/fc1912d9-dade-a759-4c5f-6aea0d0af152.jpg" [19:21:03] uuid is single image or one or more images? [19:21:19] for NA it's image based [19:21:26] or you want to get all sizes of a single image? [19:21:36] the thumb url is not 1280x1280 [19:21:50] preferably 150x150 or something [19:21:51] yeah [19:22:07] no actually css takes care of it [19:22:11] but if a user selects 1 image no need to return a thumbnail [19:22:25] css won't reduce bandwidth [19:22:29] the loop is for multi-image upload [19:22:33] when we build the gallery css makes them as small as you want [19:22:43] 03:22 css won't reduce bandwidth [19:22:49] and using the loop we can re-use all the code we made for single-image upload [19:23:07] ok zhuyifei1999_ \ [19:23:07] you literally force users to download megabytes of image then downsize it... [19:23:17] got you zhuyifei1999_ [19:23:21] makes loading suer slow [19:23:54] just make a per glam classmethod search() [19:23:55] basvb: but the loop is specific to the GLAM we are dealing with. Please correct me if I am wrong? [19:24:04] how is it? [19:24:14] the loop is based on a list of ids [19:24:20] that returns a lot of image instances [19:24:38] from a specific glam [19:24:39] the loop is not glam specific [19:24:42] what is specific about it [19:24:49] doesn't matter what the ids are [19:24:57] actually, I'm getting confused [19:24:57] input: a9946ea0-d0b4-102d-bcf8-003048976d84 [19:25:11] what is the input and expected output? [19:25:13] how do you get the thumb url generically? [19:25:33] the loop is for multi-image upload [19:25:50] for Id to thumb_url yes you write a specific function [19:26:04] and call that within the multi-image upload loop [19:26:15] for each image you call id to thumb [19:26:32] you don't do list of ids to list of thumbs [19:27:08] maybe you decide later on you want to show the thumb also for single uploads, so then separating the loop for multi-image from the thumb getting is better [19:27:42] Do you understand what I mean infobliss_? [19:28:16] yeah this is correct [19:28:44] then which part were you calling as generic? [19:28:52] this is our loop [19:29:20] ? [19:29:46] how to go from a search string to uploads [19:30:14] the main structure, the loop I wrote down twice already [19:30:57] https://github.com/infobliss/sibutest2/blob/master/glams/NationaalArchiefGLAM.py#L43 should be one Id to one image_url [19:31:06] I don't see why that should be more than 10 lines of code btw [19:31:25] and why it has to be given an url [19:31:46] yeah that will be broken into pieces [19:32:35] and why you started using xml [19:32:41] half of the code is now json half is xml [19:33:20] json is preferred? [19:33:34] json +1 [19:33:38] I prefer json [19:33:41] it's super easy to use [19:33:43] it's much more pythonic [19:33:48] yep [19:33:50] basicly json is just a dictionary [19:33:57] converts directly to a python dict [19:34:00] yeah [19:34:02] I copied from your nationaalArchief.py [19:34:18] the xml parts? [19:34:23] yeah [19:34:52] well that's my bad than, but as said before it was very hacky code, 1 day of work kind of style [19:35:00] it should not at all be seen as the correct way [19:35:48] ok do you think that the API gives json response as well for searchstring? [19:36:12] nationaalarchief.py was completely in xml [19:36:27] also the rest of the code [19:36:38] that's fine [19:36:38] well you can test it [19:36:55] I got xml response only [19:37:32] www.gahetna.nl/beeldbank-api/opensearch/?q=nederlandse kampioenschappen dammen [19:38:44] true it defaults to xml [19:38:49] and the other defaults to json [19:40:10] time to google then [19:40:16] ok [19:41:23] on the top of the xml is some info about the database [19:41:28] memorix I think it;s called [19:43:04] ok [19:45:08] http://www.gahetna.nl/beeldbank-api/zoek/?q=wageningen&output=json [19:45:19] http://www.gahetna.nl/beeldbank-api/zoek/?q=wageningen [19:45:22] works as well [19:45:26] zoek = json [19:45:30] opensearch = xml [19:45:46] http://www.gahetna.nl/beeldbank-api/ api-info [19:47:17] iok [19:47:40] zoek to be used [19:49:19] I will modify the code then [19:51:56] yes please [20:11:58] ok I'm off for today [20:12:22] tomorrow I'm here not that much, until 15:00 UTC max [20:12:27] maybe not at all [20:12:31] sunday 13:00 I'm here