Skip to content

DM-31175: Add online and offline blind solving code#27

Merged
mfisherlevine merged 28 commits intomainfrom
tickets/DM-31175
Dec 6, 2022
Merged

DM-31175: Add online and offline blind solving code#27
mfisherlevine merged 28 commits intomainfrom
tickets/DM-31175

Conversation

@mfisherlevine
Copy link
Copy Markdown
Contributor

No description provided.

self.corrFile = corrFile
self._calcScatter() # sets self.scatterPixels and self.scatterArcseconds

def _buildWcs(self):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you actually want this object to be mutable? I'm guessing not since you made buildWcs and calcScatter single-underscore and never call them outside init.

In that case, I suggest making this a frozen dataclass (from dataclasses import dataclass and add @dataclass(frozen=True)). Then, replace the existing class attributes with methods using @cached_property (from functools). The nice thing about that is that the wcs method will contain all but the last line of what is now _buildWcs, whereas plateScale will be a one-liner calling self.wcs.getPixelScale().asArcseconds(), and you won't need any single-underscore methods at all. Plus, if you really care about performance, if you never call those methods it won't bother to read the file and build the wcs in the first place.

Copy link
Copy Markdown

@taranu taranu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly minor/inane/nitpicking comments; hopefully the others are useful.

raise RuntimeError(f"Could not find {binary} in path, please install 'solve-field' and either"
" put it on your PATH or specify the full path to it in the 'binary' argument")

def _writeConfigFile(self, wide):
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just FYI, we are hopefully going to get documenteer/Sphinx support for type hints Soon and not have to write types in the numpy docstrings anymore. I'm not sure if that means we should do both in the meantime...

Copy link
Copy Markdown
Contributor Author

@mfisherlevine mfisherlevine Dec 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumably that will need mypy on everything though, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Certainly good to know though.

@mfisherlevine mfisherlevine merged commit 089f03b into main Dec 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants