Enchancing Metadata Support for SOAR #118

NucleonGodX · 2024-05-28T13:45:08Z

This Pull Request is part of Google Summer of Code (GSoC) 2024, focused on enhancing metadata support for SOAR in sunpy-soar (as discussed in issue #46 ). This initial phase involves integrating attributes from the existing sunpy.net attribute system. Specifically, join methods have been developed, and the Detector attribute has been successfully implemented for remote-sensing instruments.

nabobalis · 2024-05-28T17:23:32Z

I'll review this in a bit more detail but can you add unit tests to ensure that detector works in a query as well as the construct methods create the expected queries?

…ngle function

NucleonGodX · 2024-05-29T12:22:52Z

I've added tests, let me know if its fine.

sunpy_soar/client.py

nabobalis · 2024-05-30T18:14:36Z

sunpy_soar/client.py

+            Instrument table name.
+        """
+        instrument_name = query_item.split("=")[1][1:-1].split("-")[0].upper()
+        if instrument_name == "STIX":


I think a dict that maps the full name to the "table" name would be "cleaner" python code.

sunpy_soar/client.py

nabobalis · 2024-05-30T18:25:46Z

sunpy_soar/client.py

@@ -20,6 +20,8 @@ class SOARClient(BaseClient):
    Client to access the Solar Orbiter Archive (SOAR).
    """

+    join_needed = False


I am not sure I like the idea of this being part of the client, can this not be moved else where?

I could achieve a solution by making a basic function for it, let me know if its better!

sunpy_soar/tests/test_sunpy_soar.py

sunpy_soar/client.py

sunpy_soar/tests/test_sunpy_soar.py

ebuchlin · 2024-06-05T10:27:34Z

sunpy_soar/tests/test_sunpy_soar.py

+    level = a.Level(1)
+    res = Fido.search(instrument & time & level)
+    assert "Detector" in res[0].columns
+    assert res.file_num == 35


Testing the number of results is maybe not very robust, this number might change in SOAR.

I just followed how tests are already written in test cases. But I do understand your point!

We can check for at least 35 files?

NucleonGodX · 2024-06-10T04:13:16Z

Based on my research and the insights provided by Eric, I have implemented the wavelength functionality. I believe we should discuss the details of this implementation and further changes needed in this week's meeting.

sunpy_soar/attrs.py

nabobalis · 2024-06-12T15:49:02Z

Can you add a changelog entry.

NucleonGodX · 2024-06-12T16:45:05Z

time = a.Time("2022-03-01 00:00", "2022-03-02 00:00")
level = a.Level(1)
product = a.soar.Product("EUI-FSI174-IMAGE")
detector= a.Detector("FSI")
result = Fido.search(instrument & time & level & product & detector)
print(result)

For some reason using detector= "FSI" in query results in an API error

raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 408 Client Error:  for url: http://soar.esac.esa.int/soar-sl-tap/tap/sync?REQUEST=doQuery&LANG=ADQL&FORMAT=json&QUERY=SELECT+h1.instrument,%20h1.descriptor,%20h1.level,%20h1.begin_time,%20h1.end_time,%20h1.data_item_id,%20h1.filesize,%20h1.filename,%20h1.soop_name,%20h2.detector,%20h2.wavelength,%20h2.dimension_index+FROM+v_sc_data_item%20AS%20h1%20JOIN%20v_eui_sc_fits%20AS%20h2%20USING%20(data_item_oid)+WHERE+h1.instrument='EUI'+AND+h1.begin_time%3E='2022-03-01+00:00:00'+AND+h1.begin_time%3C='2022-03-02+00:00:00'+AND+h2.dimension_index='1'+AND+h1.level='L1'+AND+h1.descriptor='eui-fsi174-image'+AND+h2.Detector='FSI'

if the query is done without taking detector in it, it gives normal response
like for,
result = Fido.search(instrument & time & level & product)

Even with an invalid detector in the query we get the normal 0 results output.

And for other detectors also it works perfectly fine for example

time = a.Time("2022-03-01 00:00", "2022-05-02 00:00")
detector= a.Detector("HRI_EUV")
result = Fido.search(instrument & time & level & detector)

22706 Results from the SOARClient:

Instrument     Data product    Level ... Detector Wavelength
                                     ...
---------- ------------------- ----- ... -------- ----------
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       ...                 ...   ... ...      ...        ...
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0
       EUI eui-hrieuv174-image    L1 ...  HRI_EUV      174.0

sunpy_soar/tests/test_sunpy_soar.py

nabobalis · 2024-06-20T05:25:21Z

sunpy_soar/tests/test_sunpy_soar.py

+    assert "Detector" in res[0].columns
+    assert res.file_num >= 35
+
+    # test for invalid detector..


Suggested change

# test for invalid detector..

# Invalid detector

Does this need to be duplicated?

I also think it should be its own test.

sunpy_soar/tests/test_sunpy_soar.py

Co-authored-by: Nabil Freij <[email protected]>

nabobalis · 2024-06-21T20:56:22Z

sunpy_soar/tests/test_sunpy_soar.py

+    assert res.file_num == 0
+
+
+def test_search_wavelength_column_wavelength():


This feels like 3 separate tests in one? I think this is the same for the test below as well?

I would prefer if we can keep each unit test as focused as possible.
It makes fixing one easier if one part breaks but not another.

ebuchlin · 2024-06-22T09:22:13Z

Hi again, I should have mentioned that I'm the Solar Orbiter Archive Scientist. The v_sc_data_item.sensor is indexed, and so works better than v_eui_sc_fits. Also, there is a ticket to fix the lack of end time in the v_eui_sc_fits view. I can prioritise it now I know you need it.

Thanks for the advice to use this indexed column. Maybe indexed columns could be indicated in the Tables, Views, and Columns help page? I understand that timeouts can be expected when using WHERE conditions on non-indexed columns.

sunpy_soar/client.py

sunpy_soar/tests/test_sunpy_soar.py

Co-authored-by: Laura Hayes <[email protected]>

hayesla · 2024-07-04T12:46:27Z

@NucleonGodX I think the last thing to do is to not return the PHI wavelengths now until we resolve the issue with the A vs nm. So I think we should just not return it in this PR. And similarly we should not return this for SPICE. Like we discussed on our call yesterday/

So for these both, we should maybe not even return the wavelength column. Let me know if this isnt clear.

Apart from these this PR is good to go!

ebuchlin · 2024-06-21T09:58:43Z

sunpy_soar/attrs.py

+def _(wlk, attr, params):  # NOQA: ARG001
+    wavemin = attr.min.value
+    wavemax = attr.max.value
+    params.append(f"Wavemin='{wavemin}'+AND+h2.Wavemax='{wavemax}'")


Maybe I'm missing something, but isn't wavemin from h2 too?

Even if it is just parsed as a pattern in client.py, shouldn't it be consistent with wavemax?

I think as we are giving both wavemin and wavemax as a single parameter for query, I think its one of the simpler ways to implement it. If its still necessary to try something else, let me know :)

No it was just a thought about consistency between Wavemin and Wavemax.

ebuchlin · 2024-07-04T13:03:32Z

sunpy_soar/client.py

+            where_part, from_part, select_part = SOARClient.add_join_to_query(query, data_table, instrument_table)
+        else:
+            from_part = data_table
+            select_part = "*"


Do we need * here, as we select not all columns when a join is needed?

This is for instruments where we don't need the join.

ebuchlin · 2024-07-04T13:05:33Z

sunpy_soar/tests/test_sunpy_soar.py

+def test_search_wavelength_detector_column():
+    instrument = a.Instrument("EUI")
+    time = a.Time("2021-02-01", "2021-02-02")
+    level = a.Level(1)


Also a test for LL data?

Actually there is test_search_low_latency() (for MAG data; executes the query), and the new test_join_low_latency_query() (for EUI; only tests the query string, without executing it). So maybe this is enough?

yep, will reverse this change!

ebuchlin · 2024-07-04T13:20:17Z

sunpy_soar/client.py

+                # For PHI, we specify the wavemin as a parameter.
+                # This enables us to retrieve columns containing wavemin and their corresponding wavemax values.
+                # For SPICE we get data in form of wavemin/wavemax columns, but only for the first spectral window.
+                # Therefore, to make sure this data is not misleading to the user we do not return any values.
+                if "phi" in instrument_table:
+                    parameter = f"Wavemin='{wavemin_match.group(1)}'"


At the moment we don't consider wavelength for PHI and SPICE (see this comment).

nabobalis · 2024-07-06T21:55:25Z

Thanks for the reviews @ebuchlin and @hayesla.

I will run the CI and merge if it passes.

nabobalis · 2024-07-06T22:07:36Z

Thanks for the first stage of the project @NucleonGodX, you can now rebase the other two PRs to remove the commits that came from this PR.

NucleonGodX added 2 commits May 27, 2024 19:02

detector added

122e964

test fixes/minor fixes

0e3307b

added tests for detector and joins / combined join operations in a si…

70c4c32

…ngle function

detector tests added, useful comments added to contruct methods

a7f7102

nabobalis reviewed May 30, 2024

View reviewed changes

sunpy_soar/client.py Outdated Show resolved Hide resolved

nabobalis reviewed May 30, 2024

View reviewed changes

sunpy_soar/client.py Show resolved Hide resolved

nabobalis reviewed May 30, 2024

View reviewed changes

sunpy_soar/client.py Show resolved Hide resolved

nabobalis reviewed May 30, 2024

View reviewed changes

sunpy_soar/client.py Outdated Show resolved Hide resolved

nabobalis reviewed May 30, 2024

View reviewed changes

sunpy_soar/tests/test_sunpy_soar.py Outdated Show resolved Hide resolved

nabobalis reviewed May 30, 2024

View reviewed changes

sunpy_soar/tests/test_sunpy_soar.py Outdated Show resolved Hide resolved

suggestions applied

3d15042

NucleonGodX marked this pull request as draft May 31, 2024 18:09

nabobalis reviewed May 31, 2024

View reviewed changes

sunpy_soar/client.py Outdated Show resolved Hide resolved

cleaned the code, all suggestions applied

4748035

NucleonGodX marked this pull request as ready for review May 31, 2024 19:14

nabobalis reviewed May 31, 2024

View reviewed changes

sunpy_soar/client.py Outdated Show resolved Hide resolved

NucleonGodX added 4 commits June 1, 2024 08:19

comment added for join condition

f5ac3ec

pre-commit fix

8496fed

code cleanup

f4340d8

code cleanup

074bb5c

ebuchlin reviewed Jun 5, 2024

View reviewed changes

wavelength added with testcases

f9a789c

nabobalis reviewed Jun 10, 2024

View reviewed changes

sunpy_soar/attrs.py Show resolved Hide resolved

nabobalis reviewed Jun 20, 2024

View reviewed changes

sunpy_soar/tests/test_sunpy_soar.py Outdated Show resolved Hide resolved

nabobalis reviewed Jun 20, 2024

View reviewed changes

sunpy_soar/tests/test_sunpy_soar.py Outdated Show resolved Hide resolved

nabobalis reviewed Jun 20, 2024

View reviewed changes

sunpy_soar/tests/test_sunpy_soar.py Outdated Show resolved Hide resolved

nabobalis reviewed Jun 20, 2024

View reviewed changes

sunpy_soar/tests/test_sunpy_soar.py Outdated Show resolved Hide resolved

nabobalis reviewed Jun 20, 2024

View reviewed changes

sunpy_soar/tests/test_sunpy_soar.py Outdated Show resolved Hide resolved

nabobalis reviewed Jun 20, 2024

View reviewed changes

sunpy_soar/tests/test_sunpy_soar.py Show resolved Hide resolved

nabobalis reviewed Jun 20, 2024

View reviewed changes

sunpy_soar/tests/test_sunpy_soar.py Outdated Show resolved Hide resolved

NucleonGodX and others added 3 commits June 20, 2024 21:10

Apply suggestions from code review

76e1900

Co-authored-by: Nabil Freij <[email protected]>

suggestions applied

d920659

Merge branch 'main' into gsoc24-soar

d1b4510

nabobalis reviewed Jun 21, 2024

View reviewed changes

test cases simplified

6fc6921

hayesla approved these changes Jun 26, 2024

View reviewed changes

sunpy_soar/client.py Outdated Show resolved Hide resolved

sunpy_soar/tests/test_sunpy_soar.py Show resolved Hide resolved

NucleonGodX and others added 3 commits June 27, 2024 13:11

Merge branch 'main' into gsoc24-soar

f9352f1

Update sunpy_soar/client.py

52366cc

Co-authored-by: Laura Hayes <[email protected]>

tests updated

6b838c3

nabobalis approved these changes Jul 3, 2024

View reviewed changes

ebuchlin reviewed Jul 4, 2024

View reviewed changes

NucleonGodX and others added 4 commits July 4, 2024 20:11

phi and it's tests removed

183c2d8

removed low_latency detector test

645aca5

code cleanup, h2. prefix removed from attrs file

bd8bc31

Merge branch 'main' into gsoc24-soar

616f96b

NucleonGodX requested a review from ebuchlin July 6, 2024 18:43

ebuchlin approved these changes Jul 6, 2024

View reviewed changes

nabobalis merged commit f92ed86 into sunpy:main Jul 6, 2024
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enchancing Metadata Support for SOAR #118

Enchancing Metadata Support for SOAR #118

NucleonGodX commented May 28, 2024

nabobalis commented May 28, 2024

NucleonGodX commented May 29, 2024

nabobalis May 30, 2024

nabobalis May 30, 2024

NucleonGodX May 31, 2024

ebuchlin Jun 5, 2024

NucleonGodX Jun 7, 2024

nabobalis Jun 12, 2024

NucleonGodX commented Jun 10, 2024

nabobalis commented Jun 12, 2024

NucleonGodX commented Jun 12, 2024 •

edited

Loading

nabobalis Jun 20, 2024

nabobalis Jun 20, 2024 •

edited

Loading

nabobalis Jun 21, 2024 •

edited

Loading

ebuchlin commented Jun 22, 2024

hayesla commented Jul 4, 2024

ebuchlin Jun 21, 2024

ebuchlin Jun 21, 2024

NucleonGodX Jul 4, 2024

ebuchlin Jul 4, 2024

ebuchlin Jul 4, 2024

NucleonGodX Jul 4, 2024

ebuchlin Jul 4, 2024

ebuchlin Jul 4, 2024

NucleonGodX Jul 4, 2024

ebuchlin Jul 4, 2024

nabobalis commented Jul 6, 2024

nabobalis commented Jul 6, 2024 •

edited

Loading

		assert res.file_num == 0


		def test_search_wavelength_column_wavelength():

Enchancing Metadata Support for SOAR #118

Enchancing Metadata Support for SOAR #118

Conversation

NucleonGodX commented May 28, 2024

nabobalis commented May 28, 2024

NucleonGodX commented May 29, 2024

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

NucleonGodX commented Jun 10, 2024

nabobalis commented Jun 12, 2024

NucleonGodX commented Jun 12, 2024 • edited Loading

Choose a reason for hiding this comment

nabobalis Jun 20, 2024 • edited Loading

Choose a reason for hiding this comment

nabobalis Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

ebuchlin commented Jun 22, 2024

hayesla commented Jul 4, 2024

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

nabobalis commented Jul 6, 2024

nabobalis commented Jul 6, 2024 • edited Loading

NucleonGodX commented Jun 12, 2024 •

edited

Loading

nabobalis Jun 20, 2024 •

edited

Loading

nabobalis Jun 21, 2024 •

edited

Loading

nabobalis commented Jul 6, 2024 •

edited

Loading