{"action":"created","issue":{"url":"https://api.github.com/repos/hohlraum/gdsCAD/issues/24","labels_url":"https://api.github.com/repos/hohlraum/gdsCAD/issues/24/labels{/name}","comments_url":"https://api.github.com/repos/hohlraum/gdsCAD/issues/24/comments","events_url":"https://api.github.com/repos/hohlraum/gdsCAD/issues/24/events","html_url":"https://github.com/hohlraum/gdsCAD/pull/24","id":58992931,"number":24,"title":"Added support for created/modified, layerdat tuples and additional GdsIm...","user":{"login":"tom-varga","id":10090243,"avatar_url":"https://avatars.githubusercontent.com/u/10090243?v=3","gravatar_id":"","url":"https://api.github.com/users/tom-varga","html_url":"https://github.com/tom-varga","followers_url":"https://api.github.com/users/tom-varga/followers","following_url":"https://api.github.com/users/tom-varga/following{/other_user}","gists_url":"https://api.github.com/users/tom-varga/gists{/gist_id}","starred_url":"https://api.github.com/users/tom-varga/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tom-varga/subscriptions","organizations_url":"https://api.github.com/users/tom-varga/orgs","repos_url":"https://api.github.com/users/tom-varga/repos","events_url":"https://api.github.com/users/tom-varga/events{/privacy}","received_events_url":"https://api.github.com/users/tom-varga/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2015-02-26T00:13:43Z","updated_at":"2015-02-26T16:56:20Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/hohlraum/gdsCAD/pulls/24","html_url":"https://github.com/hohlraum/gdsCAD/pull/24","diff_url":"https://github.com/hohlraum/gdsCAD/pull/24.diff","patch_url":"https://github.com/hohlraum/gdsCAD/pull/24.patch"},"body":"Some gds files contain extra records that are unused by following\r\nrecords.  For example a TEXTYPE before a PATH record.  It appears that\r\ncad tools correctly ignore these records.  However, gdsCAD's use of\r\nthe **kwargs mechanism results in these unexpected records being\r\npassed to an object that doesn't expect it.  Hence, **ignore was added\r\nto a few of the class __init__ functions.\r\n\r\nSupport was added to _anchor for a consistent 2-character '{tcb}{lcr}'\r\nanchor naming convention.\r\n\r\nAdded x_reflection to Text __str__\r\n\r\nAdded support for the BGNLIB and BGNSTR created and modified fields.\r\nThe Layout and Cell classes were enhanced to support the setting and\r\naccessing of these properties.\r\n\r\nModern gds files use datatypes for most layers.  Hence get_layer is\r\nuseless especially if there isn't a get_datatype for each layer.\r\nHence get_layerdats and objects_by_layerdat accessors were added that\r\nreturn or use (layer, datatype) tuples.\r\n\r\nAdditional verbosity was added for GdsImport when verbose==2.\r\nDetails of every record where possible are also printed.\r\n"},"comment":{"url":"https://api.github.com/repos/hohlraum/gdsCAD/issues/comments/76216679","html_url":"https://github.com/hohlraum/gdsCAD/pull/24#issuecomment-76216679","issue_url":"https://api.github.com/repos/hohlraum/gdsCAD/issues/24","id":76216679,"user":{"login":"tom-varga","id":10090243,"avatar_url":"https://avatars.githubusercontent.com/u/10090243?v=3","gravatar_id":"","url":"https://api.github.com/users/tom-varga","html_url":"https://github.com/tom-varga","followers_url":"https://api.github.com/users/tom-varga/followers","following_url":"https://api.github.com/users/tom-varga/following{/other_user}","gists_url":"https://api.github.com/users/tom-varga/gists{/gist_id}","starred_url":"https://api.github.com/users/tom-varga/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/tom-varga/subscriptions","organizations_url":"https://api.github.com/users/tom-varga/orgs","repos_url":"https://api.github.com/users/tom-varga/repos","events_url":"https://api.github.com/users/tom-varga/events{/privacy}","received_events_url":"https://api.github.com/users/tom-varga/received_events","type":"User","site_admin":false},"created_at":"2015-02-26T16:56:20Z","updated_at":"2015-02-26T16:56:20Z","body":"Let me start by saying that I'm not the most experienced python programmer\nyet, so I'm more than happy to learn how to do things more pythonic.\n\nFor the unsupported records issue, I had originally thought about using a\nwrapper to check arguments which would then simply ignore unexpected\narguments.\nI think your suggestion is fine.  However, since I'm finding that some\ntools create gds files with extra records that should be ignored, I'd\nprefer not to\nsee thousands of warnings when reading in large gds files.  Maybe the\ndefault should still be to quietly ignore the extra records as cleanly and\nsafely as possible.\n\nFor the creation time questions, I agree with 1.  I tried to do it that\nway, but obviously don't understand the concept well enough yet.\nFor 2. I realized after the fact that to_gds() should be updated.  So, I\nagree.  Point 3 sounds good.  I don't see a reason to update to any time\nother than now but it could\nbe useful to somebody.\n\nAbout get_layers and get_layerdats, my needs are with the specific cell in\nquestion and not its children.  That being said, maybe both get_layers and\nget_layerdats could have an additional argument such as depth so that by\ndefault it would only return layers or layerdats to the requested depth.  I\ndon't care whether the default is like the current get_layer or\nget_layerdats behavior so long as I can specify what I want.\n\n-Tom\n\nOn Thu, Feb 26, 2015 at 4:25 AM, hohlraum <notifications@github.com> wrote:\n\n> Wow, you've been busy Tom. That's an impressive number of new additions.\n>\n> I'm wary of using **ignore to collect unsupported records. There's\n> already no typechecking on arguments (e.g. Cell_Reference('cell') doesn't\n> raise an error), so I don't like the idea of, for instance, mistyped\n> keyword arguments just disappearing. My concern is not when this is used\n> with the import function, I worry that fixing the import problem will lead\n> to more confusion in other situations.\n>\n> Better, perhaps, to clean the arguments in the _create_x wrapper. For\n> instance, change the path creation wrapper to:\n>\n> def _create_path(xy, width=1.0, layer=None, datatype=None, pathtype=0, **ignore):\n>     points = xy.reshape((xy.size // 2, 2))\n>     return Path(points, width, layer, datatype, pathtype)\n>\n> We could maybe add a warning too listing the ignored arguments.\n> ------------------------------\n>\n> The two-character anchor codes are great.\n> ------------------------------\n>\n> x_reflection for Text is good.\n> ------------------------------\n>\n> Creation/modification time. Three issues here:\n>\n>    1. I would prefer these to be attributes (i.e. with @property\n>    decorator) rather than methods. I know the code's not totally consistent,\n>    but simple information about objects is being added as an attribute rather\n>    than using getters and setters. One consequence is that there cannot be a\n>    string option, but this shouldn't be too bad since str(t) returns a\n>    formatted string for a datetime object.\n>    2. Since we now have real creation and modification dates these should\n>    be used in to_gds() rather than the placeholder now.\n>     3. Perhaps include a way of updating the modified property through\n>    update_modified() method (only allow updating to now()) , or\n>    Cell.modified=some_datetime (allow updating to any time). Which do you\n>    think is better?\n>\n> ------------------------------\n>\n> get_layerdats / objects_by_datatype\n>\n> I'm worried about the asymmetry between get_layers and get_layerdats. The\n> former returns the layers in the current cell and all children. The latter\n> returns the layers (and datatypes) of only the current cell. Is there a\n> good reason for one to behave one way, and not the other? Can/should the\n> two methods be combined into one? I'll have to think about this.\n>\n> If nothing else I should update the docstring for get_layers so that it\n> is explicit about returning its children's layer info as well.\n> ------------------------------\n>\n> Verbosity is good.\n> ------------------------------\n>\n> Valuable additions all. Let me know what you think about these comments.\n>\n> —\n> Reply to this email directly or view it on GitHub\n> <https://github.com/hohlraum/gdsCAD/pull/24#issuecomment-76146496>.\n>\n"}}