You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We have a very old version bundled in cextern (v2.1 back from 2013), I wonder whether we want to update it to a more recent one for v3.1?
Their chancelog includes several mentions of "security fixes" but I don't have enough experience with our xml utilities to be able to make an informed, quick judgement whether any of it affects us or not.
I was looking into it, but it's like a black box for me. So ideally it would be nice to be updated by someone who actually knows something about libexpat, xmls, etc.
I did look into it recently semi-accidentally and went into a 🐰 🕳 . It has obviously been refactored significantly upstream, so we need to re-discover how to build that C library. An obvious place is expat_config.h in astropy/utils/xml/src. There might be other less obvious places but I think this library is only used in astropy/utils/xml, so anyone who wants to work on this should look into:
setup scripts in top level
astropy/utils/xml
the bundled expat (which theoretically is a blind copy-and-paste from upstream?)
I noticed in iterparser.py that there is an option to do slower parsing just using python, so libexpat is not essential for things to work. The question then is whether speedy xml parsing sufficiently important that it has to be bundled? Or can it be something we recommend people to install if they deal with lots of votable files?
I'd be curious to know how much slower the Python xml parser is. That said, my gut feeling is that unless Python xml parsing is really terrible, it would perform well enough for most votable users.
Distributions build against the current version of expat, so my guess is that it should not be too hard to update ;). I did that in #8343, not sure about some changes in expat_config.h but I guess that it should work!
Activity
pllim commentedon Oct 26, 2018
Given it is already freeze time, I would say "no" to 3.1?
bsipocz commentedon Oct 28, 2018
I was looking into it, but it's like a black box for me. So ideally it would be nice to be updated by someone who actually knows something about libexpat, xmls, etc.
pllim commentedon Jan 10, 2019
I did look into it recently semi-accidentally and went into a 🐰 🕳 . It has obviously been refactored significantly upstream, so we need to re-discover how to build that C library. An obvious place is
expat_config.hinastropy/utils/xml/src. There might be other less obvious places but I think this library is only used inastropy/utils/xml, so anyone who wants to work on this should look into:astropy/utils/xmlexpat(which theoretically is a blind copy-and-paste from upstream?)I am trying my luck here by pinging @mdboom .
mhvk commentedon Jan 10, 2019
I noticed in
iterparser.pythat there is an option to do slower parsing just using python, solibexpatis not essential for things to work. The question then is whether speedy xml parsing sufficiently important that it has to be bundled? Or can it be something we recommend people to install if they deal with lots ofvotablefiles?bsipocz commentedon Jan 10, 2019
cc @tomdonaldson for answer for the
votablequestiontomdonaldson commentedon Jan 10, 2019
I'd be curious to know how much slower the Python xml parser is. That said, my gut feeling is that unless Python xml parsing is really terrible, it would perform well enough for most
votableusers.mhvk commentedon Jan 10, 2019
Sadly, unless I'm mistaken it doesn't seem we currently have any benchmarks in https://github.com/astropy/astropy-benchmarks - maybe the first goal would be to do a few quick tests and add some.
Also cc @taldcroft since he may have more sense of speed.
saimn commentedon Jan 10, 2019
Distributions build against the current version of expat, so my guess is that it should not be too hard to update ;). I did that in #8343, not sure about some changes in
expat_config.hbut I guess that it should work!taldcroft commentedon Jan 11, 2019
@mhvk - sorry, I have no sense of speed for xml parsing.