TST: Add RC testing workflow and update existing #1909

pllim · 2022-12-08T23:44:22Z

Description

This pull request is to enable any maintainer to activate the workflow dispatch to test the next astropy release candidate instead of having to create a one-off throwaway PR.

Also minor updates to existing workflows.

Blocked by

MNT: Unpin ipywidgets and voila maxversion when voila 0.4 is compatible with ipywidgets 8 #1613

Change log entry

Is a change log needed? If yes, is it added to CHANGES.rst? If you want to avoid merge conflicts,
list the proposed change log here for review and add to CHANGES.rst before merge. If no, maintainer
should add a no-changelog-entry-needed label.

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

Are two approvals required? Branch protection rule does not check for the second approval. If a second approval is not necessary, please apply the trivial label.
Do the proposed changes actually accomplish desired goals? Also manually run the affected example notebooks, if necessary.
Do the proposed changes follow the STScI Style Guides?
Are tests added/updated as required? If so, do they follow the STScI Style Guides?
Are docs added/updated as required? If so, do they follow the STScI Style Guides?
Did the CI pass? If not, are the failures related?
Is a milestone set? Set this to bugfix milestone if this is a bug fix and needs to be released ASAP; otherwise, set this to the next major release milestone.
After merge, any internal documentations need updating (e.g., JIRA, Innerspace)? 🐱

pllim · 2022-12-08T23:56:09Z

Ugh... legit deprecation warning from pre-release...

codecov · 2022-12-08T23:56:32Z

Codecov Report

Base: 91.96% // Head: 91.95% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (82da9df) compared to base (3fc9a92).
Patch coverage: 78.57% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1909      +/-   ##
==========================================
- Coverage   91.96%   91.95%   -0.02%     
==========================================
  Files         140      140              
  Lines       15315    15326      +11     
==========================================
+ Hits        14085    14093       +8     
- Misses       1230     1233       +3

Impacted Files	Coverage Δ
jdaviz/configs/imviz/plugins/catalogs/catalogs.py	`90.22% <72.72%> (-1.65%)`	⬇️
jdaviz/configs/imviz/tests/test_catalogs.py	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

pllim · 2023-01-31T14:02:46Z

Nevermind, nothing to do with voila.

Dev job failure is due to the new glue unit support: Added ability to convert units in profile viewer glue-viz/glue#2296
RC job failure is due to astroquery.sdss changes on server-side: BUG: SDSS KeyNotFoundException for query_region astropy/astroquery#2644

dhomeier · 2023-01-31T17:16:38Z

Dev job failure is due to the new glue unit support: Added ability to convert units in profile viewer glue-viz/glue#2296

6 out of 9 fixed in glue_astronomy 0.6.1; thanks for your contribution!

pllim · 2023-01-31T18:04:00Z

There are still 4 failures in the dev job. FYI.

dhomeier · 2023-01-31T23:17:48Z

Are the test_slice and test_region_spectral_spatial definitely connected to the units PR, too? I see the slice is somehow set from a wavelength with a unit, but don't have an idea how that would change its length.

pllim · 2023-02-01T13:00:22Z

don't have an idea how that would change its length

That I am not sure but astropy/specutils#999 isn't even merged yet so it can't be that. Maybe fix the two you know are tied to glue unit support first and see if the other two magically goes away. All we can do is the process of elimination.

dhomeier · 2023-02-01T15:48:07Z

Already tested on my machine; it just fixes those two (and would obviate the need for glue-astronomy>=0.6.1). 🤷

pllim · 2023-02-01T15:50:12Z

OK, let's just worry about 2/4 first and defer the other two for later. Thanks!

pllim · 2023-02-01T15:54:00Z

There is also a possibility that the other 2 would involve Jdaviz fixes to work with new unit support (e.g., calling internal API without unit no longer works) but I am just wildly guessing. A dev would have to investigate.

dhomeier · 2023-02-01T22:28:56Z

test_slice definitely indicates a regression. With the glue update, the spectrum_viewer.native_marks the wavelength is selected in are apparently just a list of indices rather than the values in Angstrom. In the CubevizExample notebook:

sv = cubeviz.app.get_viewer(cubeviz._default_spectrum_viewer_reference_name)
sv.native_marks[0].x
# glue 1.6.1
array([ 3621.59598486,  3622.42998418,  3623.26417555, ..., 10353.80559518])
# glue-dev
array([ 0, 1, 2, 3, ..., 73])

Thus apparently in Slice the last native_mark is selected.
The spectrum-viewer window in the notebook looks correspondingly broken.

astrofrog · 2023-02-02T12:09:48Z

I think I've found where the remaining failures are coming from - more soon.

astrofrog · 2023-02-02T14:00:17Z

I think glue-viz/glue#2358 fixes the remaining issues?

pllim · 2023-02-02T14:18:07Z

We'll find out in 30 mins or so. I added a commit to test your PR branch on the dev job.

pllim · 2023-02-02T14:26:15Z

Looks like I cannot test your branch here because your fork has no tags and glue-astronomy is pinning.

The conflict is caused by:
    The user requested glue-core 0.1.dev6554+gd212e9b (from git+https://github.com/astrofrog/glue.git@fix-unit-update)
    glue-astronomy 0.6.2.dev3+g71ded78 depends on glue-core>=1.6.1

dhomeier · 2023-02-02T14:31:41Z

Should be ready to merge in a minute or so, tests are looking good!

pllim · 2023-02-02T15:24:22Z

Dev job passed!

and update existing

of 3 arcmin in the pre-release of astroquery 0.4.7.dev. Also make Catalog to not crash horribly when query fails. Update catalogs test results.

because upstream number of matches can change but that is not our problem.

but on Tuesdays

pllim · 2023-02-07T17:52:57Z

RC now passes, so I removed it from pull_request trigger. Going forward, it can be manually triggered by a maintainer but it also runs weekly after merge.

I wouldn't worry about the coverage.

This is ready for review. FYI.

kecnry

Do the changes in the catalogs plugin warrant a changelog entry?

pllim · 2023-02-07T19:03:12Z

Re: changelog entry

I don't know what to write. This is upstream changes caused by astropy/astroquery#2477 but I wouldn't know enough server-side internals to write anything more useful than "I dunno, stuff changed".

pllim · 2023-02-08T22:52:02Z

There are now discussions to revert this upstream at astropy/astroquery#2659 so I don't think we need a change log. Let's wait and see. Merging.

pllim added trivial Only needs one approval instead of two no-changelog-entry-needed changelog bot directive labels Dec 8, 2022

pllim added this to the 3.2 milestone Dec 8, 2022

github-actions bot added the testing label Dec 8, 2022

This comment was marked as resolved.

Sign in to view

rosteen modified the milestones: 3.2, 3.3 Jan 4, 2023

pllim force-pushed the rc-testing branch from 6d33ecc to 33c4fc9 Compare January 24, 2023 22:30

This comment was marked as resolved.

Sign in to view

pllim force-pushed the rc-testing branch from 33c4fc9 to 321fe5e Compare January 31, 2023 12:08

pllim force-pushed the rc-testing branch from 629baf3 to 776d6d2 Compare January 31, 2023 17:35

dhomeier mentioned this pull request Jan 31, 2023

Represent None component.units as empty strings glue-viz/glue#2356

Merged

pllim force-pushed the rc-testing branch from fe826a8 to 776d6d2 Compare February 2, 2023 14:28

pllim added 2 commits February 7, 2023 11:20

TST: Add RC testing workflow

eafa22a

and update existing

Catalogs: SDSS now has max radius

7985bf8

of 3 arcmin in the pre-release of astroquery 0.4.7.dev. Also make Catalog to not crash horribly when query fails. Update catalogs test results.

pllim force-pushed the rc-testing branch from 776d6d2 to 7985bf8 Compare February 7, 2023 16:20

Relax astroquery test answer

4857100

because upstream number of matches can change but that is not our problem.

pllim marked this pull request as ready for review February 7, 2023 17:39

pllim requested review from duytnguyendtn, rosteen, javerbukh, ojustino, kecnry and bmorris3 as code owners February 7, 2023 17:39

Run RC as cron too

82da9df

but on Tuesdays

kecnry reviewed Feb 7, 2023

View reviewed changes

pllim merged commit 95c34c2 into spacetelescope:main Feb 8, 2023

pllim deleted the rc-testing branch February 8, 2023 22:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TST: Add RC testing workflow and update existing #1909

TST: Add RC testing workflow and update existing #1909

pllim commented Dec 8, 2022 •

edited

Loading

pllim commented Dec 8, 2022

codecov bot commented Dec 8, 2022 •

edited

Loading

This comment was marked as resolved.

This comment was marked as resolved.

pllim commented Jan 31, 2023 •

edited

Loading

dhomeier commented Jan 31, 2023

pllim commented Jan 31, 2023

dhomeier commented Jan 31, 2023

pllim commented Feb 1, 2023

dhomeier commented Feb 1, 2023

pllim commented Feb 1, 2023

pllim commented Feb 1, 2023

dhomeier commented Feb 1, 2023

astrofrog commented Feb 2, 2023

astrofrog commented Feb 2, 2023

pllim commented Feb 2, 2023

pllim commented Feb 2, 2023

dhomeier commented Feb 2, 2023

pllim commented Feb 2, 2023

pllim commented Feb 7, 2023

kecnry left a comment

pllim commented Feb 7, 2023

pllim commented Feb 8, 2023

TST: Add RC testing workflow and update existing #1909

TST: Add RC testing workflow and update existing #1909

Conversation

pllim commented Dec 8, 2022 • edited Loading

Description

Blocked by

Change log entry

Checklist for package maintainer(s)

pllim commented Dec 8, 2022

codecov bot commented Dec 8, 2022 • edited Loading

Codecov Report

This comment was marked as resolved.

This comment was marked as resolved.

pllim commented Jan 31, 2023 • edited Loading

dhomeier commented Jan 31, 2023

pllim commented Jan 31, 2023

dhomeier commented Jan 31, 2023

pllim commented Feb 1, 2023

dhomeier commented Feb 1, 2023

pllim commented Feb 1, 2023

pllim commented Feb 1, 2023

dhomeier commented Feb 1, 2023

astrofrog commented Feb 2, 2023

astrofrog commented Feb 2, 2023

pllim commented Feb 2, 2023

pllim commented Feb 2, 2023

dhomeier commented Feb 2, 2023

pllim commented Feb 2, 2023

pllim commented Feb 7, 2023

kecnry left a comment

Choose a reason for hiding this comment

pllim commented Feb 7, 2023

pllim commented Feb 8, 2023

pllim commented Dec 8, 2022 •

edited

Loading

codecov bot commented Dec 8, 2022 •

edited

Loading

pllim commented Jan 31, 2023 •

edited

Loading