DM-33470: Remove unused daf_persistence dependency #628
Conversation
| } | ||
|
|
||
| int countFitsHeaderCards(lsst::daf::base::PropertySet const& prop) { return prop.paramNames(false).size(); } | ||
|
|
||
| ndarray::Array<std::uint8_t, 1, 1> stringToBytes(std::string const& str) { |
There was a problem hiding this comment.
Obvious question is whether to move what's left of this file into another location because it's not really anything to do with formatters any more.
There was a problem hiding this comment.
Seems like the place to move it would be somewhere under afw.table. It's not really as connected with typehandling.
There was a problem hiding this comment.
I see that these two functions are only used for tables so I suppose I could try to move them somewhere. Unless you have a good idea where I'm inclined to merge and get the easy win of daf_persistence avoidance.
There was a problem hiding this comment.
I don't understand. If you've removed them, but they are still "used for tables", why isn't something breaking?
There was a problem hiding this comment.
We are talking about the two functions that are left in the file. They seem to be used by WCS for example to convert to bytes to stuff into a table and then convert back to strings to read out of a table.
There was a problem hiding this comment.
Ah, ok. I'd lean towards moving them into the source files that use them, if it's just a couple of source files (not headers), but if it's much work at all, then leaving them where they are is probably best, since reducing churn is good, too.
There was a problem hiding this comment.
No. There are four places:
include/lsst/afw/typehandling/python.h
178: std::string stringRep = formatters::bytesToString(record.get(keys.bytes));
202: record->set(keys.bytes, formatters::stringToBytes(o));
src/math/TransformBoundedField.cc
112: auto frameSetStr = formatters::bytesToString(record.get(keys.frameSet));
137: record->set(keys.frameSet, formatters::stringToBytes(getTransform().writeString()));
src/geom/SkyWcs.cc
98: std::string stringRep = formatters::bytesToString(record.get(keys.wcs));
385: record->set(keys.wcs, formatters::stringToBytes(writeString()));
src/geom/Transform.cc
232: std::string stringRep = formatters::bytesToString(record.get(keys.bytes));
244: record->set(keys.bytes, formatters::stringToBytes(writeString()));
There was a problem hiding this comment.
I think the right thing to do might have been to look at the common usage in the above four locations. It's always getting or setting in a BaseRecord, so that might be the place to move it.
This also removes a daf_persistence dependency.
timj
force-pushed
the
tickets/DM-33470
branch
from
0d3afdf
to
ba43998
Compare
No description provided.