[15:20:54] @basvb: https://github.com/infobliss/sibutest2/blob/master/app.py#L147 [15:22:13] only then multiple images upload will take place for AM [15:22:16] I need to get these values here. Please suggest how I may retrieve them. [17:42:35] hi [17:42:53] how is the progress infobliss? [17:54:37] hi basvb [17:54:45] please check the logs [17:55:05] will do [17:56:14] I don't understand the issue exactly [17:56:42] you can't get it to return the wikitext, image_url and title> [17:56:43] ? [17:58:11] infobliss? [17:58:34] I can't access these objects since they were already created here https://github.com/infobliss/sibutest2/blob/master/app.py#L118 [17:59:52] wasn't that the whole discussion we had a week or so ago about how to send variables through the app? [18:00:12] how did you tackle it for NA, because I'd guess you would have the same issue [18:01:11] and diving in on that, sorry to say so but the current way app.py is structured is not scalable at all, you're doing everything completely different for the GLAMs [18:01:42] I need to have two classmethod functions each glam [18:02:01] please expand on that [18:02:05] one is get_thumbnail() [18:02:14] which is already written by you [18:02:31] it's there, but I believe it's not classmethod (however I'm not an expert on that) [18:02:45] and the other one should return the identifier per image [18:03:08] ? [18:03:13] you are sending it the identifiers [18:03:28] sending whom? [18:03:31] so why would it need to return them [18:04:07] if you instantiate AmsterdamMuseum() you instantiate it with an identifier [18:04:14] yes [18:04:23] AmsterdamMuseum(3) [18:04:24] for example [18:04:33] so why would it need to return the identifier? [18:04:42] and if it has to just make a simple get function [18:04:45] get identifier() [18:04:50] return self.priref [18:05:19] I do not wish to instantiate any object inside during the processing of the first form [18:05:30] true [18:05:40] so how do you tackle this for NA? [18:05:47] rather I intend to instantiate them once the multiple uploading is done [18:06:18] for NA there is one method which should ideally be a classmethod called gallery_builder() [18:06:22] you realize that app.py is the location to have code to communicate (between front-end and back-end) [18:06:44] yes [18:06:44] yes gallery builder should not be in NA glam specific code [18:06:49] it should be generic code [18:06:56] https://github.com/infobliss/sibutest2/blob/master/app.py#L111 [18:07:07] yes that is agreed [18:07:28] but to build the gallery we need to classmethods in each GLAM class as mentioned earlier [18:07:33] you really have to take a good look at the code and see what is generic and what is specific [18:07:38] *two [18:07:42] currently there is generic code in NA-class [18:07:47] and specific code in app.py [18:07:51] neither should happen [18:08:15] sure [18:08:26] app.py should contain no more than a list/dictionary (or other data structure) with specific information on the glams [18:08:58] to build a gallery you don't need code at the glam side [18:09:08] you should build the gallery outside of the glam code [18:09:22] the code for building a gallery for NA is the same as for building a gallery for AM [18:09:33] the only thing you need are the thumbnails [18:09:51] so you write a generic function [18:09:58] it is given a list of ids [18:10:01] how about the identifiers? [18:10:02] and a glam name [18:10:10] they are needed for object instantiation [18:10:28] then for that glam you call the necessary functions [18:10:39] check license and get_thumbnail [18:10:59] def gallery_builder(ids, glam): [18:11:02] for id in ids: [18:11:17] glam(id).check_license() [18:11:31] if glam(id).check_license(): [18:11:51] good_ids.append(id) [18:12:12] thumbnails.append(glam(id).get_thumbnail() [18:12:16] something like that [18:12:28] and after that you construct the thumbnails based on those lists [18:12:53] you need some more checks and what I wrote here isn't 100% correct but I hope you grasp the idea [18:13:50] did I explain it ok or are there any unclarities I should expand on? [18:14:10] I have issues about instantiating the objects. Do you agree about my plan of object instantiation? [18:14:31] I don't know what your plan is [18:15:33] I need to have two classmethod functions each glam. one is get_thumbnail() and the other one should return the identifier per image. [18:15:53] then I can instantiate objects per image upload [18:16:18] and all will be generically done [18:16:27] making the flow same for all glams [18:16:48] you already know the identifier of the image when you communicate with the glam instance [18:17:05] so you don't need a function that can return those identifiers [18:17:13] it won't work [18:17:14] No once the first form is submitted this info will be lost [18:17:23] yes so you have to keep track of it [18:17:27] the identifiers info I mean [18:17:39] there is a function for searchstring to identifiers btw [18:17:45] that's the one that gets called first [18:17:53] yeah I used it [18:18:10] https://github.com/infobliss/sibutest2/blob/master/app.py#L114 [18:19:18] these identifiers should really be send forward in app.py [18:19:21] either via session [18:19:26] or maybe you can change flask.render_template('image_gallery.html', glam_name = 'AM', image_list = image_list) [18:19:33] to flask.render_template('image_gallery.html', glam_name = 'AM', image_list = image_list, id_list) [18:19:43] and then in that template you call [18:19:45] ok that can be done [18:19:57] https://github.com/infobliss/sibutest2/blob/master/app.py#L125 [18:20:18] but before you do that please compare your app.py with https://github.com/toollabs/video2commons/blob/master/video2commons/frontend/app.py (the example we were using) [18:20:24] do you see the difference? [18:22:40] yeah [18:22:50] it has several methods per route [18:23:03] that's not the difference I'm aiming at [18:23:17] not much computation [18:23:49] indeed, app.py in video2commons is only communicating between things [18:23:56] between front and back-end [18:24:09] you are also taking a large part of the backend in your app.py [18:24:19] you shouldn't do that [18:25:05] for example the code where you go from ids to thumbnails and all of that (gallery buildings) [18:25:11] should be a separate code [18:25:23] it's not part of any specific glam (nor the generic glam class) [18:25:28] but also not from app.py [18:25:45] we need more logical structure in the code to make it usable [18:25:54] that brings me onto another point [18:26:00] we are nearing the end of this project [18:26:17] the two intermediate deadlines you all of a sudden realized there was still a lot to do [18:26:48] so for this time: there is still a lot to do, and in 2 weeks the product should be finished 99% [18:27:08] after that we can ask some people to comment or do some small things [18:27:26] but you should see 22/24 august as the deadline [18:28:00] and on that deadline I'd like to see the following: we've optimized user-flow, the user should understand how the tool works from the text accompanied with it [18:28:05] "there is still a lot to do" <= what all do you include here? [18:28:09] the code should be easily appendable [18:28:24] that means no specific functions in generic parts [18:28:25] no hacks [18:28:33] no generic functions in specific parts [18:28:55] and documentation should be clear and available [18:29:03] both incode documentation [18:29:10] as well as some outcode documentation [18:29:29] no more print statements on places they shouldn't be [18:30:18] somebody who knows python should be able to add a new glam without needing personal help/or one of us fixing something [18:31:16] I think once we make a decision on how to send data forward in app.py we would make a lot of progress [18:31:30] or maybe you can change flask.render_template('image_gallery.html', glam_name = 'AM', image_list = image_list) <= Is this ok to do? [18:31:57] If yes then this can be followed for each glam [18:32:16] If no how do we move this data forward? [18:32:49] what are you asking me exactly [18:33:18] " or maybe you can change flask.render_template('image_gallery.html', glam_name = 'AM', image_list = image_list) <= Is this ok to do" [18:33:21] that is the comment I made? [18:33:24] corret? [18:33:44] Is this ok to pass the id_list forward via the render_template? [18:34:03] I'm not sure if that is the correct way, it seems possible to me [18:34:24] what you wanted on glam-side I believe was technically impossible [18:34:30] yeah it is possible [18:34:33] or at least very hacky [18:34:45] so maybe ask zhuyifei1999_ what he things about that method [18:34:53] I'm not sure if that is the correct way, it seems possible to me <= it is possible to do [18:35:02] ok [18:35:20] in the mean time, I think it's a prime opportunity to align NA-code a bit with how AM-code looks [18:35:29] and try to put generic functions outside of it [18:35:29] of course [18:35:33] remove print statements [18:35:35] comment code [18:35:36] etc. [18:35:48] and then try to have the multiple image uploading done before sunday [18:36:00] so that then we can discuss whether there is anything that can be done for the last week [18:36:18] I will restructure NA code on priority [18:36:20] so we have empty space for the last week to plan the most needed things [18:36:25] I have tested other parts all works fine [18:36:30] good [18:36:54] I can also push the code to toollabs so that you may test [18:36:57] and please, really listen to what we are saying, the print statements and code commenting I feel we've told you more than 10 times [18:37:30] sure please bear with me [18:37:55] I think I had requested the print statements to be there till the merging of AM so that it helps in testing [18:38:40] use debug in your next project, it's not the proper way to do it [18:38:57] it refers to print [18:39:05] you mean debug in an IDE? [18:39:05] https://github.com/infobliss/sibutest2/commit/20b8b523eb9a192b47e6b7753c9239b26e364dc1#diff-7e4874f648e3d826a911e1ab8037b588R24 [18:39:11] that was nice and clean [18:39:16] I work in ubuntu with a text editor [18:39:35] or using debuglogging [18:41:04] https://github.com/infobliss/sibutest2/commit/20b8b523eb9a192b47e6b7753c9239b26e364dc1#diff-7e4874f648e3d826a911e1ab8037b588R299 [18:41:07] was that causing bugs? [18:41:15] if so why only change that one and not the others [18:41:45] https://github.com/infobliss/sibutest2/commit/20b8b523eb9a192b47e6b7753c9239b26e364dc1#diff-7e4874f648e3d826a911e1ab8037b588R298 is the proper place to typecast it [18:41:48] self.parameters['medium'] = '' ? [18:41:56] you mean this one? [18:42:07] yeah it gave keyError once [18:42:25] no I don't mean that one [18:42:31] R298 and R299 [18:42:37] https://github.com/infobliss/sibutest2/commit/20b8b523eb9a192b47e6b7753c9239b26e364dc1#diff-7e4874f648e3d826a911e1ab8037b588R328 was my next point [18:42:48] it is already set as '' [18:43:10] by wikitemplates.art_photo_parameters [18:43:16] they were not comparible [18:43:22] https://github.com/infobliss/sibutest2/commit/20b8b523eb9a192b47e6b7753c9239b26e364dc1#diff-7e4874f648e3d826a911e1ab8037b588R37 [18:43:34] so please fix it in R298 [18:43:43] because the same will go wrong for the other cases [18:43:47] okk [18:44:53] this gave AttributeError: 'AmsterdamMuseumGLAM' object has no attribute 'categories'? [18:45:17] also same error for parameters [18:45:29] that was an issue yesterday [18:46:20] what gave that error? [18:46:31] what refers "this" to in that sentence? [18:46:36] never mind it's gone [18:46:39] it was some json parsing related issue KeyError [18:46:51] https://github.com/infobliss/sibutest2/commit/20b8b523eb9a192b47e6b7753c9239b26e364dc1#diff-7e4874f648e3d826a911e1ab8037b588R37 [18:47:00] 'this' means this [18:47:19] that was already there, you didn't change it [18:47:31] I mean the medium= '' gets done there already [18:47:37] so redoing it is redundant code [18:47:55] no I meant R37 [18:48:00] if it doesn't work that's very strange and it's better to find the real cause than to "hack" it like that [18:48:04] yes I know [18:48:10] but R37 isn't changed [18:48:20] so how did that give errors [18:48:50] it was some json parsing related issue KeyError [18:49:12] the json was not generating the data properly [18:49:26] but anyways not an issue anymore [18:49:30] we are miscommunicating [18:50:22] are you asking about medium= ' '? [18:50:29] yes [18:50:44] I think I may remove that [18:50:57] It gave that key medium not found or something [18:51:05] I don't recall correctlyu [18:53:00] strange [18:53:33] https://github.com/infobliss/sibutest2/blob/master/libraries/infobox_templates.py#L6 [18:53:37] there it is [18:54:44] Yeah imo you may like to ignore this for now [18:54:56] this is a resolved issue [18:55:41] I will test this after removing this and will reflect in the next commit [18:59:31] it's ok, but I wouldn't call it resolved. It's something that grasped my interest as I think the code should be as clean and pure as we can get it [19:00:06] of course you were right to pick that point [19:00:29] I just tested it [19:00:43] worked fine without that line [19:04:13] nice [19:06:00] also I think since have an OOP approach we can't help doing object initiation etc in the app.py itself [19:06:13] making the app.py look bigger than necessary [19:07:43] I disagree [19:08:00] OOP is keeping the code shorter [19:08:09] we now have 2 glams [19:08:12] but imagine 1000 [19:08:14] 100 [19:08:44] currently code such as https://github.com/infobliss/sibutest2/blob/master/app.py#L83-103 is per glam [19:08:48] that means 10 lines per glam [19:08:56] = 1000 lines if we have 100 glams [19:09:08] so that should be avoided to be done per glam [19:09:56] I get the point [19:10:16] All I am saying is https://github.com/infobliss/sibutest2/blob/master/app.py#L84 line is unavoidable per glam [19:10:22] Is that correct? [19:11:12] no, you can avoid that [19:11:19] by putting it into a dictionary [19:11:42] either a dictionary in app.py, even better is likely to have a separate config-like file [19:12:16] you can then reuse that in both the javascript/html code and the python code [19:12:32] what are key and values in the dict you are proposing? [19:12:45] if you introduce a new glam you just add it to that config-file and it gets loaded [19:12:48] depends on what you need [19:13:09] a name and an id? [19:13:25] no that dictionary only has info on the glams [19:13:54] 'Nationaal Archief' = NationaalArchiefGLAM.NationaalArchiefGLAM() [19:15:06] the glam name is the key and the glam class __init__ is the value? [19:15:22] something like that yes [19:15:30] but I'm not too familiar with it [19:15:39] the internet might be a better source [19:16:00] alright I will see [19:22:15] basvb are you there? [19:22:23] yes [19:22:44] for 100 glams we should ideally have a loop to avoid 100 if condtions [19:23:13] for that we need some iterable object to iterate one [19:23:26] not a loop but a dictionary [19:23:26] do you get me? [19:23:39] yeah yeah [19:23:43] the dict is there [19:23:48] you have to design everything with 100-1000s glam in mind [19:23:56] to make things generic [19:24:46] on top of the dict we have to decide what action to take depending on an if condition [19:25:20] we definitely do not wish to check like https://github.com/infobliss/sibutest2/blob/master/app.py#L84 [19:25:34] comparing with the glam name [19:26:13] if glam in dict: [19:26:23] dict[glam] [19:26:43] ok sorry [19:26:50] list is already iterable [19:27:48] nice thanks it was a much needed change for being scalable [19:27:56] I appreciate it very much :) [19:33:44] np, it's where we have to go to [19:33:49] I'm off for tonight [19:33:56] have to finish something and then sleeptime [19:34:02] bb and speak to you tomorro [19:34:46] see you :) [21:01:20] what is ment by sending data forward? [21:01:22] *meant [21:01:52] (and sorry I really can't be online at 1-3 am)