Week12 #26

sholmbo · 2021-03-23T18:54:03Z

I had to add two keys to the config.ini under [api],

address = https://flows.phys.au.dk/api
catalog = https://flows.phys.au.dk/api

to enable the custom api.

And @emirkmo is to blame for the switch to spaces :p

astroalign is sometimes returning false matches so we remove it for now until we can fix this. Also some changes to the wcs_checking loop to catch whether references left the frame or got near the edge after the new wcs.

This reverts commit 3e435ad.

@todo

@todo: this dependency can be removed with smarter numpy or astropy table usage.

emirkmo · 2021-04-09T11:04:36Z

Let's get this merged if we can. Any additions can come in future PRs. This is large enough..

rhandberg · 2021-04-14T10:45:49Z

This will cause us to have to drop support for Python 3.6. This will cause problems for some of the systems currently running the automatic parts of the code, until we can get them upgraded. What is the reason for the strict "astropy >= 4.2" requirement?

rhandberg · 2021-04-14T10:47:00Z

Please install all dependencies (pip install -r requirements.txt) and run "flake8" and fix any errors it finds.

rhandberg · 2021-04-14T11:09:42Z

Why is it needed to have the API URL in the config file... This sounds to me like a way to open a security hole... I change that config to point to something nasty and bad things happen... And it will break existing setups if it is not defined

rhandberg · 2021-04-14T11:15:51Z

Please the following lines in the beginning of all files:

#!/usr/bin/env python3
# -*- coding: utf-8 -*-
"""
What does the code in this file do?

.. codeauthor:: Your Name <youremail@example.com>
"""

rhandberg

Could we rename the "localweb" or "web" to something more describing? Like "local target plotting interface" or whatever...

flows/coordinatematch/wcs.py

flows/load_image.py

requirements.txt

web/api/catalogs.json

web/api/datafiles.json

web/api/sites.json

web/api/targets.json

emirkmo · 2021-04-14T13:38:08Z

This will cause us to have to drop support for Python 3.6. This will cause problems for some of the systems currently running the automatic parts of the code, until we can get them upgraded. What is the reason for the strict "astropy >= 4.2" requirement?"

I think my nearest neighbor WCS correction code needs Astropy 4.2, and we intend to have it as a backup. So we should upgrade all dependencies to python >3.6 and astropy >=4.2, but let's do that later. Possibly, we change my code to not need astropy 4.2. It's just that they added a nice WCS solver in 4.2 that works well for matched pairs.

rhandberg · 2021-04-21T08:23:51Z

flows/epsfbuilder/epsfbuilder.py

+import photutils.psf
+
+
+class EPSFBuilder(photutils.psf.EPSFBuilder):


Same thing as for the "WCS" object... Lets give it a new name so there is no confusion with the original EPSFBuilder... FlowsEPSFBuilder maybe?

Since we're just copying and extending it, I think we should keep the same name. Then in the future if we move back to photutils EPSFBuilder, we don't have to refactor the code.

rhandberg · 2021-04-21T08:25:20Z

flows/load_image.py

@@ -84,6 +85,9 @@ def load_image(FILENAME):
 		image.image = np.asarray(hdul[0].data, dtype='float64')
 		image.shape = image.image.shape

+		image.head = hdr
+		image.exthdu = [hdu.copy() for hdu in hdul[1:]]


Is this used anywhere? It is an expensive operation if not used for anything and we might as well just use the HDUList from the original FITS file if it is needed.

rhandberg · 2021-04-21T08:25:43Z

flows/load_image.py

@@ -84,6 +85,9 @@ def load_image(FILENAME):
 		image.image = np.asarray(hdul[0].data, dtype='float64')
 		image.shape = image.image.shape

+		image.head = hdr


Let's rename this to "header" instead of "head"

rhandberg · 2021-04-21T08:27:28Z

flows/photometry.py

 from astropy.table import Table, vstack
 from astropy.nddata import NDData
 from astropy.modeling import models, fitting
-from astropy.wcs.utils import proj_plane_pixel_area
+from astropy.wcs.utils import proj_plane_pixel_area, fit_wcs_from_points
 from astropy.time import Time

 warnings.simplefilter('ignore', category=AstropyDeprecationWarning)


Is this warnings.simplefilter needed twice? (properly not)
The reason it was here originally was that photutils would throw warnings when importing... Is that still the case with the newer version?

We need this for now yes, especially with our version fixing of Astropy/Photutils

rhandberg · 2021-04-21T08:31:20Z

flows/coordinatematch/__init__.py

@@ -0,0 +1,2 @@
+from .coordinatematch import CoordinateMatch # noqa


Be specific on which warnings you are disabling. Never use the "disable all warnings" unless there is a REALLY good reason for it.

These are error F401 but we should ignore F401 by default on all init.py files.

Rasmus Comments, flake8 things. Correct Branch

codecov · 2021-04-21T13:23:39Z

Codecov Report

Merging #26 (d18dd43) into devel (847a5d2) will increase coverage by 0.51%.
The diff coverage is 12.29%.

❗ Current head d18dd43 differs from pull request most recent head 730fb73. Consider uploading reports for the commit 730fb73 to get more accurate results

@@            Coverage Diff             @@
##            devel      #26      +/-   ##
==========================================
+ Coverage   16.81%   17.33%   +0.51%     
==========================================
  Files          22       28       +6     
  Lines        1564     1979     +415     
==========================================
+ Hits          263      343      +80     
- Misses       1301     1636     +335

Impacted Files	Coverage Δ
flows/load_image.py	`7.05% <0.00%> (-0.34%)`	⬇️
flows/photometry.py	`10.58% <6.46%> (+2.37%)`	⬆️
flows/wcs.py	`11.29% <11.29%> (ø)`
flows/coordinatematch/coordinatematch.py	`11.97% <11.97%> (ø)`
flows/coordinatematch/wcs.py	`25.00% <25.00%> (ø)`
flows/epsfbuilder/epsfbuilder.py	`39.13% <39.13%> (ø)`
flows/coordinatematch/__init__.py	`100.00% <100.00%> (ø)`
flows/epsfbuilder/__init__.py	`100.00% <100.00%> (ø)`
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 847a5d2...730fb73. Read the comment docs.

Fix PEP8 and missing )

missing operator flake8

emirkmo · 2021-04-21T18:18:30Z

Ok it’s ready to pull in

rhandberg · 2021-04-22T10:19:34Z

Okay, I will merge this, but I will be doing some cosmetic changes afterwards

emirkmo and others added 30 commits January 8, 2021 17:07

implement wcs checking with sep and astropy

c1e5e97

wcs_checking_fix

22e10ae

Adjust WCS with identified stars

3e435ad

Remove astroalign & improve wcs checking loop

fffc31a

astroalign is sometimes returning false matches so we remove it for now until we can fix this. Also some changes to the wcs_checking loop to catch whether references left the frame or got near the edge after the new wcs.

Auto stash before merge of "wcs_fix" and "emirkmo/wcs_fix"

eb8e028

Revert "Adjust WCS with identified stars"

145c6e9

This reverts commit 3e435ad.

Merge remote-tracking branch 'rasmus/devel' into devel

88dc233

CoordinateMatch for WCS

fb4a019

Merge devel into wcs_fix

dc3fc44

Merge branch 'wcs_fix' of github.com:emirkmo/flows into emir_wcs_fix

62cc7bd

Coordinate Match

71c0640

Added docstrings

eb9bc01

Making the pipeline work with Simon's new changes

5818798

Merge remote-tracking branch 'origin/devel' into emir_wcs_fix

9b7363a

Bugfix

10158b4

Bug Fix in final clean

3ee000e

Weave in wcs correction, bugfix,

af65ca6

Merge remote-tracking branch 'origin/devel' into emir_wcs_fix

2941574

add pandas to requirements

ffc790c

@todo: this dependency can be removed with smarter numpy or astropy table usage.

Add keep_fixed_diff arg

67fa7c5

missing return statement

487a1b4

log rsq values for debug

8e353b6

typo

b6a51cf

testing wcs.py

99c42e9

lower timeout error

47f39d5

fix masked array

5a0720f

add force hidpi option

b448b8f

Timeout Time as parameter

511b380

add timeout par as command line arg

52b9479

timeout par bug

5c79ebe

Bunch of stuff

4328391

rhandberg added the enhancement New feature or request label Apr 14, 2021

rhandberg requested changes Apr 14, 2021

View reviewed changes

emirkmo mentioned this pull request Apr 14, 2021

Astropy >= 4.2 will cause us to drop support for Python 3.6. #27

Closed

-web +pep8

d18dd43

sholmbo requested a review from rhandberg April 14, 2021 15:06

rhandberg reviewed Apr 21, 2021

View reviewed changes

emirkmo and others added 2 commits April 21, 2021 15:11

Rasmus Comments, flake8 things.

249f44e

Merge pull request #2 from emirkmo/week12_2

c415fd6

Rasmus Comments, flake8 things. Correct Branch

emirkmo and others added 7 commits April 21, 2021 15:34

Fix missing )

045e6f5

flake8 changes

5beb8dc

Merge pull request #3 from emirkmo/week12_2

6033838

Fix PEP8 and missing )

Changed astropy requirement to 4.1

11d2f8b

Merge remote-tracking branch 'sholmbo/week12' into week12

2febc2f

missing operator flake8

9546238

Merge pull request #4 from emirkmo/week12_2

730fb73

missing operator flake8

rhandberg merged commit 843304e into SNflows:devel Apr 22, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Week12 #26

Week12 #26

sholmbo commented Mar 23, 2021

emirkmo commented Apr 9, 2021

rhandberg commented Apr 14, 2021

rhandberg commented Apr 14, 2021

rhandberg commented Apr 14, 2021

rhandberg commented Apr 14, 2021

rhandberg left a comment

emirkmo commented Apr 14, 2021

rhandberg Apr 21, 2021

emirkmo Apr 21, 2021

rhandberg Apr 21, 2021

emirkmo Apr 21, 2021

rhandberg Apr 21, 2021

emirkmo Apr 21, 2021

rhandberg Apr 21, 2021

emirkmo Apr 21, 2021

rhandberg Apr 21, 2021

emirkmo Apr 21, 2021

codecov bot commented Apr 21, 2021 •

edited

Loading

emirkmo commented Apr 21, 2021

rhandberg commented Apr 22, 2021

		import photutils.psf


		class EPSFBuilder(photutils.psf.EPSFBuilder):

		@@ -0,0 +1,2 @@
		from .coordinatematch import CoordinateMatch # noqa

Week12 #26

Week12 #26

Conversation

sholmbo commented Mar 23, 2021

emirkmo commented Apr 9, 2021

rhandberg commented Apr 14, 2021

rhandberg commented Apr 14, 2021

rhandberg commented Apr 14, 2021

rhandberg commented Apr 14, 2021

rhandberg left a comment

Choose a reason for hiding this comment

emirkmo commented Apr 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 21, 2021 • edited Loading

Codecov Report

emirkmo commented Apr 21, 2021

rhandberg commented Apr 22, 2021

codecov bot commented Apr 21, 2021 •

edited

Loading