Esa gaia gaiapcr 1290 include the new data link linking parameter #2859
Conversation
|
There exist a failing test that is not part of this PR
|
There was a problem hiding this comment.
Thank you for the PR @cosmoJFH.
I left a couple of inline comments. Besides those, it would be great not to include any reformatting changes, like the line wraps in unrelated PRs. Now it's just adds a couple 10s of extra diff, but in bigger PRs it can be rather distruptive.
Do get this merged, would you consider showing the usage in the narrative docs? Either by adding a new example, or modifying one of the existing ones.
Also, having a non monkeypatched test would be useful, so we actually test that the code works with the server, as expected.
astroquery/gaia/core.py
Outdated
| output_file = os.path.join(os.getcwd(), temp_dirname, | ||
| downloadname_formated) |
There was a problem hiding this comment.
We do let longer lines in astroquery (max-line-length = 120). It is supposed to be picked up from the config files, but I'm happy to add new sections that your editor/IDE would respect.
There was a problem hiding this comment.
Sorry for the mistake, I misconfigured my pycharm IDE: I set the max line length to 79 characters. I have reverted the formatting changes
| By default, all the identifiers are considered as source_id | ||
| SOURCE_ID: the identifiers are considered as source_id |
There was a problem hiding this comment.
Why to have SOURCE_ID as the default then, instead of None?
There was a problem hiding this comment.
Yes, you are right it is more convenient to set the default value to SOURCE_ID
|
We have included a new section to the gaia documentation, devoted to the datalink service. Note that it does not include any reference to the new parameter linking_parameter, since this information must be included in the gaia help (https://www.cosmos.esa.int/web/gaia-users/archive) beforehand by the gaia team as ESAC. |
There was a problem hiding this comment.
Some suggestions of wordsmithing and cleanups in the docs, otherwise this is good to go.
@cosmoJFH - if you're happy with the suggestions, just give me the go-ahead, and I can commit the changes along with fixes to the docs build and merge.
|
Hi @bsipocz we are happy with the changes you suggested for the documentation. |
Codecov Report
@@ Coverage Diff @@
## main #2859 +/- ##
=======================================
Coverage 66.51% 66.52%
=======================================
Files 235 235
Lines 18096 18101 +5
=======================================
+ Hits 12037 12041 +4
- Misses 6059 6060 +1
... and 3 files with indirect coverage changes 📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today! |
bsipocz
force-pushed
the
ESA_gaia_GAIAPCR-1290_Include_the_new_DataLink_linking_parameter
branch
from
f7bcd11
to
99a9b8a
Compare
bsipocz
force-pushed
the
ESA_gaia_GAIAPCR-1290_Include_the_new_DataLink_linking_parameter
branch
from
99a9b8a
to
8e4142d
Compare
|
Thank you @cosmoJFH! |
Hi, my name is Jorge Fdez, and I work as a software developer at ESAC (gaia mission). The new parameter linking_parameter can be used in the datalink service, in order to provide an additional meaning to the source identifiers: source_id, transit_id and image_id. This parameter is optional, in order to be backward compatible, and therefore, if the parameter is not set, the source identifiers are considered as source_id,
These changes are related to the jira ticket https://s2e2.cosmos.esa.int/jira/browse/GAIAPCR-1290.
@esdc-esac-esa-int