get_obs_products method supports product_type parameter as string or list

[davidglt]

The get_obs_products method now checks the input argument product_type with the method _check_list_strings and allows using strings or lists

[pep8speaks]

Hello @davidglt! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-05-13 08:16:59 UTC

[jespinosaar]

cc @esdc-esac-esa-int

[davidglt]

Hi there!
The last commit 61b8b14 only modifies from the file in the main branch the lines to include this PR information, but maybe I have something more wrong :)
Cheers,
David

[jespinosaar]

Hi @davidglt , please rebase your branch. It seems there is a conflict in CHANGES.rst file.

[davidglt]

Hi there!
After rebase, we have the same problem with the CHANGES.rst file

Cheers,
David

[bsipocz]

I'm not exactly sure what went wrong with the rebase here, I suspect it might not been rebased on the correct branch. Given that there are new conflicts I do a rebase again and will report back the command history I will have used.

[bsipocz]

Given that there were a lot of incorrect edits for the changelog, I removed those commits during the rebase and instead added a single clean on once the rest has been rebased. Here are the commands:

Make sure you have the astropy/astroquery repo added as a git remote (I call this astropy in my setup, but it maybe upstream in yours):

$ ~/munka/devel/astroquery [ESA_jwst-extend_get_obs_products|✚ 1…1⚑ 14] $ git remote -v
astropy	git@github.com:astropy/astroquery.git (fetch)
astropy	git@github.com:astropy/astroquery.git (push)
bsipocz	git@github.com:bsipocz/astroquery.git (fetch)
bsipocz	git@github.com:bsipocz/astroquery.git (push)
esdc-esac-esa-int	git@github.com:esdc-esac-esa-int/astroquery (fetch)
esdc-esac-esa-int	git@github.com:esdc-esac-esa-int/astroquery (push)

Have a fresh version of the main repo:

git fetch --all --prune

Interactive rebase on astropy/main. This will automatically get rid of the ~80 commits that you pulled in during the previous rebase. Also, in the interactive mode an editor will pop up. I removed all commits related to the changelog. The rebase was clean, I didn't need to resolve any conflicts, but if there are any, you read the instructions and do --continue until all commits are successfully rebased.

git rebase -i astropy/main

Once the rebase is done, I edited the changelog file and added that commit.

Now, the important part (which might have been the step that didn't worked previously) is to force push the branch back to github. I call the ESAC fork push esdc-esac-esa-int, you may have it as origin.

git push esdc-esac-esa-int HEAD:ESA_jwst-extend_get_obs_products -f

[bsipocz]

Please add a test that covers the list case for product_type input. Also see the inline comments as I suspect the a bug, but also suggest a shorter solution for the enhancement.

[bsipocz]

I would strongly advise calling the parameter something else than list to avoid overriding the list()

[bsipocz]

a single string is iterable and this will return True, is that the logic you want to do here?

[bsipocz]

this function is only used in this one place, so I would instead suggest something in-place here along the line of the following few line. Arguably this doesn't gracefully handle the case when the input list is not full of strings, but the docs calls for strings, so duct-typeing should be sufficient. And with this the extra private function is not needed either.

Suggested change

	if self.__check_list_strings(product_type):
	if type(product_type) is list:
	tap_product_type = ",".join(str(elem) for elem in product_type)
	else:
	tap_product_type = product_type
	else:
	tap_product_type = None
	if isinstance(product_type, list):
	tap_product_type = ",".join(str(elem) for elem in product_type)
	elif isinstance(product_type, str):
	tap_product_type = product_type
	else:
	tap_product_type = None

[davidglt]

Brigitta, thank you very much for your comments and patience!
I was not using the --force in the push, my fault :)
About this method "check_list_strings" is used as in other submodules (ex. hubble), but you’re right, in jwst no, I can delete it and check the entries directly in "get_obs_products".
Cheers,
David

[bsipocz]

Also, @davidglt - if you add the email address you made the commits with to your github profile, these commits will properly show up as your contributions.

[davidglt]

Hi Brigitta, I am working to implement a new test for the product_type parameter as string or list