Add Developer guide for further development. #147
Conversation
|
I've just done initial push of settings things up, will update it all in the next few days. |
|
I've been quite busy with my college stuff, I'll push my further changes today. |
docs/dev_guide/query.rst
Outdated
| level = a.Level(2) | ||
| distance = a.soar.Distance(0.28 * u.AU, 0.30 * u.AU) | ||
|
|
||
| result = Fido.search(instrument & level & distance) |
There was a problem hiding this comment.
We should check this result is output.
docs/dev_guide/tables.rst
Outdated
| Depending on the use case, you might need to determine the type of join (e.g., inner join, outer join, left join) and what | ||
| specific tables or columns should be displayed to the user. In some cases, conditional logic may be required, where | ||
| certain attributes in the search query trigger specific join operations or column selections. |
There was a problem hiding this comment.
| Depending on the use case, you might need to determine the type of join (e.g., inner join, outer join, left join) and what | |
| specific tables or columns should be displayed to the user. In some cases, conditional logic may be required, where | |
| certain attributes in the search query trigger specific join operations or column selections. | |
| Depending on the use case, you might need to determine the type of join (e.g., inner join, outer join, left join) and what specific tables or columns should be displayed to the user. | |
| In some cases, conditional logic may be required, where certain attributes in the search query trigger specific join operations or column selections. |
Any examples of the conditional logic?
There was a problem hiding this comment.
okay, I'll add.
There was a problem hiding this comment.
The explanation I provided was specifically in the context of handling joins with the FoV (Field of View) table, where certain columns are returned when the user includes FoV in their search query. However, since the implementation details of this feature are still under discussion, it might be best to omit this section for now.
Once the FoV pull request (PR) is finalized and the implementation becomes clear, I am eager to update the guide with the new capabilities and features that will be introduced.
docs/dev_guide/tables.rst
Outdated
| Finally, ensure that the ``_do_search`` method is updated to reflect these changes. This method should handle any | ||
| additional columns or conditional logic for attribute-specific queries, ensuring that the appropriate data is returned | ||
| to the user. |
There was a problem hiding this comment.
| Finally, ensure that the ``_do_search`` method is updated to reflect these changes. This method should handle any | |
| additional columns or conditional logic for attribute-specific queries, ensuring that the appropriate data is returned | |
| to the user. | |
| Finally, ensure that the ``_do_search`` method is updated to reflect these changes. | |
| This method should handle any additional columns or conditional logic for attribute-specific queries, ensuring that the appropriate data is returned to the user. |
Can we not link to the do_search method?
Co-authored-by: Nabil Freij <[email protected]>
docs/dev_guide/working.rst
Outdated
| "SELECT+*+FROM+v_sc_data_item+WHERE+instrument='EPD'+AND+begin_time>='2021-02-01+00:00:00'+AND+begin_time<='2021-02-02+00:00:00'+AND+level='L1'+AND+descriptor='epd-epthet2-nom-close'" | ||
| # Or with a Join | ||
| "SELECT+h1.instrument, h1.descriptor, h1.level, h1.begin_time, h1.end_time, h1.data_item_id, h1.filesize, h1.filename, h1.soop_name, h2.detector, h2.wavelength, h2.dimension_index+" | ||
| "FROM+v_sc_data_item AS h1 JOIN v_eui_sc_fits AS h2 USING (data_item_oid)+WHERE+h1.instrument='EUI'+AND+h1.begin_time>='2021-02-01+00:00:00'+AND+h1.begin_time<='2021-02-02+00:00:00'+AND+"" | ||
| "h2.dimension_index='1'+AND+h1.level='L1'+AND+h1.descriptor='eui-fsi174-image'" |
There was a problem hiding this comment.
I would split the code block in 2, write the queries as basic ADQL (no URL-encoding of spaces to pluses, no double quotes in each line; code block language could be SQL), and transform the comment into regular text in between both code blocks.
|
|
||
|
|
||
| ``sunpy-soar`` constructs the query based on the specified criteria, then generates a URL to interact with the SOAR API. | ||
| This is done by using the Table Access Protocol (TAP), a widely adopted standard in the astronomical community for accessing large datasets. |
There was a problem hiding this comment.
Mention that queries are in ADQL? (with reference to the standard
| The ``sunpy-soar`` library currently supports data retrieval from both science and low-latency data tables, such as ``v_sc_data_item`` and ``v_ll_data_item``. | ||
| Additionally, it provides support for join tables associated with remote instruments, such as ``v_eui_sc_fits``. |
There was a problem hiding this comment.
These tables and their columns are described in the Tables, Views, and Columns SOAR help page (add link).
| It performs a general query based on the provided parameters, retrieving the data that matches the criteria specified. | ||
|
|
||
| ``doQueryFilteredByDistance``: This method is employed when a distance parameter is included in the search query. | ||
| Unlike ``doQuery``, this method filters the entire query based on the specified distance value. |
There was a problem hiding this comment.
I don't understand what is different. What does "entire query" mean? Or is that the entire database?
docs/dev_guide/query.rst
Outdated
|
|
||
| ``doQueryFilteredByDistance``: This method is employed when a distance parameter is included in the search query. | ||
| Unlike ``doQuery``, this method filters the entire query based on the specified distance value. | ||
| The time attribute is not required when using ``doQueryFilteredByDistance``, as the time is internally calculated based on the provided distance. |
There was a problem hiding this comment.
This implies that there is a computation of time ranges corresponding to a distance range. I am not sure the query works like this internally at SOAR, and in any case the way it works internally at SOAR is not relevant for the sunpy-soar user or developer.
docs/dev_guide/query.rst
Outdated
| ``doQueryFilteredByDistance``: This method is employed when a distance parameter is included in the search query. | ||
| Unlike ``doQuery``, this method filters the entire query based on the specified distance value. | ||
| The time attribute is not required when using ``doQueryFilteredByDistance``, as the time is internally calculated based on the provided distance. | ||
| The distance value is appended to the end of the query using ``&DISTANCE(dmin, dmax)``, where ``dmin`` and ``dmax`` are Astropy units of distance. |
There was a problem hiding this comment.
Suggestion:
The distance range of values is appended to the end of the query using ``&DISTANCE(dmin, dmax)``, where ``dmin`` and ``dmax`` are Astropy quantities representing distances.| Unlike ``doQuery``, this method filters the entire query based on the specified distance value. | ||
| The time attribute is not required when using ``doQueryFilteredByDistance``, as the time is internally calculated based on the provided distance. | ||
| The distance value is appended to the end of the query using ``&DISTANCE(dmin, dmax)``, where ``dmin`` and ``dmax`` are Astropy units of distance. | ||
| These values must fall within the range of 0.28 AU to 1.0 AU; otherwise, the query will not return any results. |
There was a problem hiding this comment.
For DISTANCE(0 * u.au, .5 * u.au) for example (or even a negative value for dmin...), I would expect the filtering to be done on everything below 0.5AU, I don't expect the query to systematically give no result. So if this true, this could be a SOAR misfeature that we can submit to SOAR team.
There was a problem hiding this comment.
According to this comment SOAR does indeed not behave as I expect.
There was a problem hiding this comment.
@nabobalis I think this was the only comment left to be addressed?
There was a problem hiding this comment.
There are a range of "outdated" comments from Eric but they don't seem to be addressed in the text?
docs/dev_guide/query.rst
Outdated
| Here the query's "REQUEST" type to "doQueryFilteredByDistance", which is a special method that filters the entire query based on the specified distance value. | ||
| This method is used when a distance parameter is included in the search query, and the time attribute is not required. |
There was a problem hiding this comment.
The second sentence and the end of the first sentence duplicate something that has already been written.
|
@nabobalis some of the functions are private like _can_handle_query so how should we have sphinx reference links for them? |
They would need to be added to sunpy docs for the links to work. |
|
@NucleonGodX, do you have any time in the near future to account for Eric's review comments? |
|
Hey all, just a quick comment that the search by distance works up to 1.02 AU, not just 1.0 AU. Thanks :) |
Yea sure I'll address them this week. |
This is with reference to issue (#146 ), For creating a developer guide for future developers about how further development can be done in sunpy-soar, and what is the current workflow and how can further tables or metadata can be added.