New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue 1801 old scr files #1820

Merged
merged 19 commits into from Mar 22, 2016

Conversation

Projects
None yet
3 participants
@doutriaux1
Member

doutriaux1 commented Feb 10, 2016

@aashish24 @danlipsa @sankhesh 99% fixes for reading VERY old vcs script files, but there are a few tweaks in there for setting line attributes using an actual line object or its name, please review.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 10, 2016

@@ -877,6 +883,10 @@ def checkLineType(self, name, value):
hvalue = 'long-dash'
elif (queries.isline(value) == 1):
hvalue = value.name
elif value in vcs.elements["line"]:
self.linecolor = vcs.elements["line"][value].color[0]

This comment has been minimized.

@sankhesh

sankhesh Feb 10, 2016

Contributor

If vcs.elements["line"][value] returns None, these lines would cause a crash.

If you think that will never happen, please ignore.

This comment has been minimized.

@doutriaux1

doutriaux1 Feb 10, 2016

Member

yes it should never happen vcs.elements are auto-populated when creating vcs objects

@sankhesh

This comment has been minimized.

Contributor

sankhesh commented Feb 10, 2016

Apart from the one comment, LGTM.

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 16, 2016

@chaosphere2112 @aashish24 can you please merge. Thanks

vals.append(float(V))
atts[a] = vals
f.style = atts.get("fais", f.style)
if code.find("(") > -1: # ok with have the keywords speeled out

This comment has been minimized.

@chaosphere2112

chaosphere2112 Feb 17, 2016

Contributor

spelled*

This comment has been minimized.

@doutriaux1

doutriaux1 Feb 17, 2016

Member

you don't know about speeling keywords? You should looking into it it's most useful 😉

atts[a] = vals
f.style = atts.get("fais", f.style)
if code.find("(") > -1: # ok with have the keywords speeled out
for a in ["faci", "fasi", "fais", "faoi", "vp", "wc", "x", "y"]:

This comment has been minimized.

@chaosphere2112

chaosphere2112 Feb 17, 2016

Contributor

Since you apparently know what these magic strings refer to, mind documenting them?

This comment has been minimized.

@doutriaux1

doutriaux1 Feb 17, 2016

Member

I do but will document them for the benefit of future generations

@@ -36,7 +36,6 @@ def process_src(nm, code):
for a in ["ltyp", "lwsf", "lci", "vp", "wc", "x", "y"]:

This comment has been minimized.

@chaosphere2112

chaosphere2112 Feb 17, 2016

Contributor

Again, documentation of the magic strings would be appreciated 😄

@@ -106,7 +141,8 @@ def dumpToJson(obj, fileout, skipped=[
f.close()
for etype in associated.keys():
for asso in associated[etype]:
if asso is not None and asso not in vcs._protected_elements[etype]:
if asso is not None and asso not in vcs._protected_elements[

This comment has been minimized.

@chaosphere2112

chaosphere2112 Feb 17, 2016

Contributor

This is not a nice linebreak.

This comment has been minimized.

@doutriaux1

doutriaux1 Feb 18, 2016

Member

autopep8 did it.

@@ -138,7 +174,8 @@ def process_src_element(code):
i = code.find("(")
nm = code[:i]
code = code[i + 1:-1]
try:
# try:
if 1:

This comment has been minimized.

@chaosphere2112
@@ -89,6 +89,8 @@ def process_src(nm, code):
try:
setattr(gm, nm, sp[1])
except:
if nm == "line":
print "And it fails"

This comment has been minimized.

@chaosphere2112

chaosphere2112 Feb 17, 2016

Contributor

Debug comment?

This comment has been minimized.

@doutriaux1

doutriaux1 Feb 17, 2016

Member

most definitely!

doutriaux1 added some commits Feb 17, 2016

Update vector.py
remove debug comment
Update fillarea.py
typo and magic keyword meaning explained when possible
Update line.py
keywords doc
@chaosphere2112

This comment has been minimized.

Contributor

chaosphere2112 commented Feb 24, 2016

LGTM; does it pass all tests?

@doutriaux1

This comment has been minimized.

Member

doutriaux1 commented Feb 24, 2016

@chaosphere2112 running it now

doutriaux1 added a commit that referenced this pull request Mar 22, 2016

@doutriaux1 doutriaux1 merged commit d919754 into master Mar 22, 2016

0 of 3 checks passed

continuous-integration/kitware-buildbot/uvcdat-garant-linux-release/ Build started
Details
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build is in progress
Details

@doutriaux1 doutriaux1 deleted the issue_1801_old_scr_files branch Mar 22, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment