curtacircuitos / pcb-tools

Tools to work with PCB data (Gerber, Excellon, NC files) using Python.
Apache License 2.0
279 stars 135 forks source link

Optional modifiers for circle and macro incorrect #27

Open mguthaus opened 9 years ago

mguthaus commented 9 years ago

Is the current github repo of your tools in a working condition? I cannot get the example to run. It seems to give errors on the regular expressions for the aperture defines like this:

%?(?PAD)D(?P\d+)(?PC)[,]?(?P[^,])?%?

The error is: raise error, v # invalid expression sre_constants.error: nothing to repeat

I think there are some problems with the number of modifiers. According to the rs274 specification, there must always be a diameter modifier for a circle, so It should be: %?(?PAD)D(?P\d+)(?PC),*%?

Similar, this is true for the macro type.

I am emaiing you a patch... Please let me know if I am mistaken, however, the example now runs correctly.

Best, Matt

mguthaus commented 9 years ago

Looks like the formatting messed up the regex examples... anyhow, I sent you a patch.

phsilva commented 9 years ago

Matt, what example are you trying to run and getting this error? I just tried the cairo example on examples folder and it works as expected.

I added support for the non compliant way of using a circle aperture without modifiers because I have a bunch of Gerbers in this way. While you must provide modifiers for circle, you are allowed to have zeroed circles and some CADs just ignore the modifier altogether.

Aperture macro usage do not require modifiers after the name as well.

mguthaus commented 9 years ago

I ran the example in the example folder (examples/cairo_example.py) on OSX with Python 2.7.5.

While I understand that some tools may output malformed gerber files without these modifiers (they are required in the specification), then the regex would need to be improved for 0 or more modifiers. Right now, it wasn't even parsable in the version of Python I mentioned.

On Thu, May 14, 2015 at 2:46 PM, Paulo Henrique Silva < notifications@github.com> wrote:

Matt, what example are you trying to run and getting this error? I just tried the cairo example on examples folder and it works as expected.

I added support for the non compliant way of using a circle aperture without modifiers because I have a bunch of Gerbers in this way. While you must provide modifiers for circle, you are allowed to have zeroed circles and some CADs just ignore the modifier altogether.

Aperture macro usage do not require modifiers after the name as well.

— Reply to this email directly or view it on GitHub https://github.com/curtacircuitos/pcb-tools/issues/27#issuecomment-102178950 .

Matthew Guthaus Associate Professor, Computer Engineering University of California Santa Cruz http://www.soe.ucsc.edu/~mrg http://vlsida.soe.ucsc.edu/

phsilva commented 9 years ago

I am using Python 2.7.9 (just updated, worked on 2.7.6 as well) on OS X.

regex right now looks like this (https://github.com/curtacircuitos/pcb-tools/blob/master/gerber/rs274x.py#L147)

AD_CIRCLE = r"(?P<param>AD)D(?P<d>\d+)(?P<shape>C)[,]?(?P<modifiers>[^,]*)?"

which do support optional (0 or more, using '?' in the last group). we do run a tests using this 2.7, 3.3 and 3.4 at https://travis-ci.org/curtacircuitos/pcb-tools.

try this case here just to see what is going on:

import re
AD_CIRCLE = r"(?P<param>AD)D(?P<d>\d+)(?P<shape>C)[,]?(?P<modifiers>[^,]*)?"
re.match(AD_CIRCLE, "ADD10C0").groups()
re.match(AD_CIRCLE, "ADD10C").groups()

for me, the output here is:

>> import re
>>> AD_CIRCLE = r"(?P<param>AD)D(?P<d>\d+)(?P<shape>C)[,]?(?P<modifiers>[^,]*)?"
>>> re.match(AD_CIRCLE, "ADD10C0").groups()
('AD', '10', 'C', '0')
>>> re.match(AD_CIRCLE, "ADD10C").groups()
('AD', '10', 'C', '')
>>>
mguthaus commented 9 years ago

import re AD_CIRCLE = r"(?PAD)D(?P\d+)(?PC)[,]?(?P[^,]*)?" re.match(AD_CIRCLE, "ADD10C0").groups() Traceback (most recent call last): File "", line 1, in File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/re.py", line 137, in match return _compile(pattern, flags).match(string) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/re.py", line 242, in _compile raise error, v # invalid expression sre_constants.error: nothing to repeat re.match(AD_CIRCLE, "ADD10C").groups() Traceback (most recent call last): File "", line 1, in File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/re.py", line 137, in match return _compile(pattern, flags).match(string) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/re.py", line 242, in _compile raise error, v # invalid expression sre_constants.error: nothing to repeat

On Thu, May 14, 2015 at 3:22 PM, Paulo Henrique Silva < notifications@github.com> wrote:

I am using Python 2.7.9 (just updated, worked on 2.7.6 as well) on OS X.

regex right now looks like this ( https://github.com/curtacircuitos/pcb-tools/blob/master/gerber/rs274x.py#L147 )

AD_CIRCLE = r"(?PAD)D(?P\d+)(?PC)[,]?(?P[^,]*)?"

which do support optional (0 or more, using '?' in the last group). we do run a tests using this 2.7, 3.3 and 3.4 at https://travis-ci.org/curtacircuitos/pcb-tools.

try this case here just to see what is going on:

import re AD_CIRCLE = r"(?PAD)D(?P\d+)(?PC)[,]?(?P[^,]*)?" re.match(AD_CIRCLE, "ADD10C0").groups() re.match(AD_CIRCLE, "ADD10C").groups()

for me, the output here is:

import re>>> AD_CIRCLE = r"(?PAD)D(?P\d+)(?PC)[,]?(?P[^,]*)?">>> re.match(AD_CIRCLE, "ADD10C0").groups() ('AD', '10', 'C', '0')>>> re.match(AD_CIRCLE, "ADD10C").groups() ('AD', '10', 'C', '')>>>

— Reply to this email directly or view it on GitHub https://github.com/curtacircuitos/pcb-tools/issues/27#issuecomment-102186703 .

Matthew Guthaus Associate Professor, Computer Engineering University of California Santa Cruz http://www.soe.ucsc.edu/~mrg http://vlsida.soe.ucsc.edu/

mguthaus commented 9 years ago

I agree now that it seems the regular expression should be well formed upon further investigation. (I'm new to Python re, but am well-versed in Perl.) This is quite odd though since I'm using the standard system Python.

On Thu, May 14, 2015 at 3:23 PM, Matthew Guthaus mrg@ucsc.edu wrote:

import re AD_CIRCLE = r"(?PAD)D(?P\d+)(?PC)[,]?(?P[^,]*)?" re.match(AD_CIRCLE, "ADD10C0").groups() Traceback (most recent call last): File "", line 1, in File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/re.py", line 137, in match return _compile(pattern, flags).match(string) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/re.py", line 242, in _compile raise error, v # invalid expression sre_constants.error: nothing to repeat re.match(AD_CIRCLE, "ADD10C").groups() Traceback (most recent call last): File "", line 1, in File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/re.py", line 137, in match return _compile(pattern, flags).match(string) File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/re.py", line 242, in _compile raise error, v # invalid expression sre_constants.error: nothing to repeat

On Thu, May 14, 2015 at 3:22 PM, Paulo Henrique Silva < notifications@github.com> wrote:

I am using Python 2.7.9 (just updated, worked on 2.7.6 as well) on OS X.

regex right now looks like this ( https://github.com/curtacircuitos/pcb-tools/blob/master/gerber/rs274x.py#L147 )

AD_CIRCLE = r"(?PAD)D(?P\d+)(?PC)[,]?(?P[^,]*)?"

which do support optional (0 or more, using '?' in the last group). we do run a tests using this 2.7, 3.3 and 3.4 at https://travis-ci.org/curtacircuitos/pcb-tools.

try this case here just to see what is going on:

import re AD_CIRCLE = r"(?PAD)D(?P\d+)(?PC)[,]?(?P[^,]*)?" re.match(AD_CIRCLE, "ADD10C0").groups() re.match(AD_CIRCLE, "ADD10C").groups()

for me, the output here is:

import re>>> AD_CIRCLE = r"(?PAD)D(?P\d+)(?PC)[,]?(?P[^,]*)?">>> re.match(AD_CIRCLE, "ADD10C0").groups() ('AD', '10', 'C', '0')>>> re.match(AD_CIRCLE, "ADD10C").groups() ('AD', '10', 'C', '')>>>

— Reply to this email directly or view it on GitHub https://github.com/curtacircuitos/pcb-tools/issues/27#issuecomment-102186703 .

Matthew Guthaus Associate Professor, Computer Engineering University of California Santa Cruz http://www.soe.ucsc.edu/~mrg http://vlsida.soe.ucsc.edu/

Matthew Guthaus Associate Professor, Computer Engineering University of California Santa Cruz http://www.soe.ucsc.edu/~mrg http://vlsida.soe.ucsc.edu/

phsilva commented 9 years ago

yes, that is odd indeed, let me dig a little bit more here. I'll get the same 2.7.5 version here to test.

hamiltonkibbe commented 9 years ago

I can't get it to reproduce on 2.7.6 or later on OSX.

Looks like it may be caused by http://bugs.python.org/issue18647. That check (in sre_compile.py) was removed on 2013/08/19 and 2.7.6 was released 2013/11/10: https://hg.python.org/cpython/rev/7b867a46a8b4/ which seems to match up with what we're seeing here

Regex wizardry isn't really my forte so I'm not sure what the fix is... It seems to be an issue when the compiler decides that something before an asterisk or question mark etc. could potentially be empty