Any features that would help maintaining the parts repository? #88
Comments
|
hello vanepp, thank you for this! fritzing really needs help with all the parts and the part contribution. there is a little beginning for a test and validation tool based on travis-ci tests and go code. the aim is to make automatic contribution more easy. maybe you could find a way to integrate your test+features to this tool(s) thank you so far and greetings |
|
I'll certainly have a look and see what I can do, I'm not really a developer per se I was a system administrator for 20+ years before retiring. I started this project in perl (which I have used for 25 years or more) with little success then switched to python (which is new to me) and made more progress in a couple of days than the past month in perl so my quality probably isn' t production grade but it works so far. I still need to figure out how to use github to put the scripts up for use which I haven't done yet. That's probably the place to start I expect and go from there. Peter Van Epp |
|
While I couldn't figure out how to use travis, I have eventually (a year after I started) gotten a parts checking python script working far enough to release and available here: https://github.com/vanepp/FritzingCheckPart It was posted to the Fritzing forums a couple of days ago and is alpha code (as I'm so far the only one that has used it) but it finds lots of issues in the core parts bin (sometimes by being overly strict). It also performs its main task of removing the trailing px from font-size commands, change white silkscreen to black and inline style commands (which Fritzing doesn't like in bendable legs) when editing svg files with Inkscape. If there are things you would like it to do that it currently doesn't I'd be happy to try and add them. Peter Van Epp |
|
Hi @vanepp , |
|
The version on github is stable and works. At a quick look I wasn't able to figure out how Travis worked and thus haven't tried to integrate it. One of the few other folks using it did modify it for use with Travis and as I recall he needed to change the return code on warnings (which aren't fatal) from the current 1 to 0 (errors which cause Fritzing to fail return 2 I think). More of a problem is likely to be that it wants to modify the svg files to correct some of the stuff that Inkscape sticks in for CSS compatability such as px on font-size and style commands (which Fritzing doesn't fully support). That is fine for my use, because it corrects some problems and leaves me with the corrected part, but I don't think that will work for what I think Travis does, which is only check the part complies without changing anything. I'd assume it won't be able to modify the files in the repo and likely would need to change to just flag those as errors because they will cause problems in Fritzing. It should be easy enough to fork a version and change it to only complain about things like style and px in font size though. |
|
Yes, modifying the parts does not make sense via travis, but I bet the fixes done by the script could make for some awesome error messages. Instead of applying the fix, just print it out as suggestion along with the error message. |
|
OK, I'll work on that. The script needs some corrections to deal with copper0 on the same level as copper1 (it currently objects to that as an error, but needs to verify the format is correct instead) and some others. I'm thinking that FritzingCheckPart.py will only check and complain for use with travis-ci and FritzingFixPart.py will do the current local corrections if you want it to fix the errors it can (same code base, just different options) to have the best of both worlds. Peter |
|
Issue is too generic, and stale. I think we should close it? |
|
I am not up on what a validation script does during CI, but would expect that it does a regression test. Which would be against the whole repository contents, not just the changed files. With the possibility that a single svg is used by multiple parts, it would not (without extra logic) have any way of knowing which parts and svg files to limit the checking to. Which means that FIRST the existing parts (whole existing repo) need to pass the test. So yes, the issue is generic, and FCP is not currently suitable. Possibly a new issue (project) to add CI tests to travis, in a staged manner, in parallel with part cleanup? Only add tests that will pass for the current repo, then add more as cleanup of specific cases is completed. A somewhat standard (unit test) path: notice a problem, write a test case that fails for that case, fix the problem, verify the test now passes, push the fix and test case. With a separate path for (a tool) reporting 'lint' issues, that are suspicious, but not failures. |
|
A CI is already in place, checks are run on all parts. However, we only run the script written in go, not yet the python script, as that tries to modify the parts. |
|
Even if it was not doing any modifications, FCP would not currently work for CI, because a lot of the existing parts do not pass. I started another (python) script that (initially) only checks for a very small subset of issues, and it still found a lot of bad parts. Including one fzp in obsolete that is not even valid xml. Some of that was reported on the slack channel, while asking about what was safe to change without also needed to obsolete parts. |
|
A useful line for checking fzp and svg changes (and any kind of xml) Can this be made into a gihub action which renders the diff at a PR, similar to the deepcode bot? For automated CI tests, see also PR #270 , I started fitting some of the scripts for CI purposes. |
|
"A useful line for checking fzp and svg changes (and any kind of xml) I'll have a poke at this. Currently one problem with Inkscape is it changes the export order (I expect this is python, which outputs dictionaries in either random, or on some plan known to itself order, that changes every time you output :-) .) On my list is to write a sort routine on the final prettyprinted output xml (which at that point is a block of text) to sort the elements in to a consistent order so the output xml will (hopefully!) be possible to diff. You will probably need to prettyprint the input svg first to get a consistent base, but that shouldn't be a big deal. If this tool can already deal with Inkscape (which it may be able to), so much the better. Peter |
|
Python lxml can maintain the sort order. If you use minidom , then you need
at least python 3.8 to keep the sorting.
Have you seen the recent commits, there i use a list to skip files, maybe
the same pattern can be applied to FritzingCheckPart
vanepp <notifications@github.com> schrieb am Fr. 30. Okt. 2020 um 23:16:
… "A useful line for checking fzp and svg changes (and any kind of xml)
git difftool -y --extcmd=/usr/bin/xmldiff"
I'll have a poke at this. Currently one problem with Inkscape is it
changes the export order (I expect this is python, which outputs
dictionaries in either random, or on some plan known to itself order, that
changes every time you output :-) .) On my list is to write a sort routine
on the final prettyprinted output xml (which at that point is a block of
text) to sort the elements in to a consistent order so the output xml will
(hopefully!) be possible to diff. You will probably need to prettyprint the
input svg first to get a consistent base, but that shouldn't be a big deal.
If this tool can already deal with Inkscape (which it may be able to), so
much the better.
That said I'm still poking at FritzingCheckPart, I have a version that
only checks and complains (and does the fixes it can if run as
FritzingFixPart.py) but partly creeping featureitis and partly laziness /
lack of skill is making progress slow. As Phil mentioned there will then be
a big cleanup of core parts needed as many parts fail now. I'll look at
replacing the error message generation with a function call (it currently
appends the text to a text blob.) With that in place it would be trivial to
replace the Error calls (which create a non zero return code) with a
Warning call (which emits a 0 return code) to allow the current parts to be
fixed up over time (and once they are fixed change the Warning back to
Error.) This pr is rather old and I have no objection to closing it, and
opening a new one when I manage to make some progress on FritzingCheckPart.
There are still a few (and probably some I haven't found yet!) false
positives to fix up. They were OK for manual use, but not acceptable (at
least without a way to over ride them) in the input check stream.
Peter
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#88 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABRROSOWGK3GJJTSKV4DHQLSNM3MPANCNFSM4DDP3QOQ>
.
|
|
"Python lxml can maintain the sort order. " Do you know the parameter to do this? FritzingCheckPart uses lxml but I didn't know it would do that. I'll have a look at the lxml docs and see if I can find what it wants to activate it. Edit: The name did it all, there is an example in the lxml FAQ. I'll try that out and see if it will fix up the Inkscape output. Peter |
|
lxml had a bug last year (also affects python 3.5): https://bugs.launchpad.net/lxml/+bug/1838252 |
While this isn't an issue per se but I don't know of any other way to contact el-j :-) and I know he reads these. I'm currently working on a python script that takes as input an fpz file and uses the lxml parser library to parse both that and the associated svg files to do a number of things; convert the silkscreen from white to black so the silkscreen is visible in Inkscape, convert style commands to inline xml (as bendable legs at least don't work with style commands), remove the px from font size commands (Fritzing reduces the font size to 0 when such a part is editied) and copy inherited stoke commands down to the final xml as the python script that creates the gerber drill file can't inherit from the xml and makes wrong drill sizes (all of these are currently working). As well I'm going to verify that the connectors specified in the fpz exist in the associated svg. It however occurs to me that there may be things that would help el-j that I don't know about so I figured I'd ask if there are requests while I'm still working on the script if there are such issues.
The text was updated successfully, but these errors were encountered: