Fix fits mask merge behaviour #24
Conversation
bennahugo
commented
Mar 1, 2023
bennahugo
commented
- Fixes Merge does not work #23 -- switch to logical operations
- Creates switch to make ever more restrictive masks during merging by logical_and vs default of logical_or
- Fixes #23 -- switch to logical operations - Creates switch to make ever more restrictive masks during merging by logical_and vs default of logical_or
| @@ -145,6 +147,8 @@ def main(): | |||
| help='Merge in one or more masks or region files') | |||
| parser.add_argument('--subtract', dest='subtract', metavar="MASK(s)|REG(s)", nargs='+', | |||
| help='Subract one or more masks or region files') | |||
| parser.add_argument('--merge_and', dest='mergeand', action='store_true', | |||
There was a problem hiding this comment.
In general, I would prefer --merge-and over --merge_and -- but thinking about it another minute, it's not really a "merge" operation, it's an "intersect"... I think the CLI would be cleaner if this was defined as a separate top-level operation rather than a flag on top of "merge":
parser.add_argument('--merge', dest='merge', metavar="MASK(s)|REG(s)", nargs='+',
help='Merge in one or more masks or region files')
parser.add_argument('--subtract', dest='subtract', metavar="MASK(s)|REG(s)", nargs='+',
help='Subract one or more masks or region files')
parser.add_argument('--intersect', dest='intersect', metavar="MASK(s)|REG(s)", nargs='+',
help='Intersect with one or more masks or region files')
There was a problem hiding this comment.
Why don't we just implement explicit, top level logical OR / AND / XOR operations? I think the meaning of "intersect" is less clear. Wrote a simple script for this ages ago and have used it more times than I can count, so it's probably worth adding this general functionality to breizorro...
https://github.com/IanHeywood/oxkat/blob/master/tools/image_logic.py
There was a problem hiding this comment.
Set theory language vs. boolean logic language, take your pick...
But then add_argument does allow multiple option names, so why not have it both ways:
parser.add_argument('--merge', '--or', dest='merge', metavar="MASK(s)|REG(s)", nargs='+',
help='Merge (logical-OR) one or more masks or region files')
parser.add_argument('--intersect', '--and', dest='intersect', metavar="MASK(s)|REG(s)", nargs='+',
help='Intersect (logical-AND) one or more masks or region files')Just not sure what --subtract is in boolean language? It's not XOR (and is XOR conceivably useful for masks anyway?)
There was a problem hiding this comment.
I think subtract is subtract...
There was a problem hiding this comment.
Just not sure what --subtract is in boolean language?
NAND? 🤔
is XOR conceivably useful for masks anyway?
Probably not (that's not not NOT) actually.
|
My two cents on this is that masks are boolean in general usage. Add and subtract of masks makes no sense to me... I vote +1 on the boolean operator definitions |
Well what we call "subtract" now is "subtract" or "difference" in set theory terms, and the equivalent of |
== NAND, no? |
NOT ( |
|
I've also just realised that NAND would give us an ocean of 1s wherever both masks were 0...! |
Precisely my point! Incidentally, {NAND} forms a complete basis set for boolean algebra, in the sense that you can express any boolean function in terms of NANDs and NANDs only. Which makes it great for chip manufacturers, but not necessarily great for breizorro.. |
|
Yup, you can build anything with enough 7400 ICs. |
