Skip to content

Bugfix for pagesize and page parameters in catalogs.query_criteria()#3065

Merged
bsipocz merged 2 commits into
astropy:mainfrom
snbianco:ASB-25227-catalogs-page-size
Jul 14, 2024
Merged

Bugfix for pagesize and page parameters in catalogs.query_criteria()#3065
bsipocz merged 2 commits into
astropy:mainfrom
snbianco:ASB-25227-catalogs-page-size

Conversation

@snbianco

Copy link
Copy Markdown
Contributor

Fix bug so that the pagesize and page parameters work properly for Catalogs.query_criteria().

Also added to unit test cases to check that these are working properly.

@snbianco snbianco marked this pull request as ready for review July 10, 2024 17:52

@bsipocz bsipocz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renaming an argument should be done with the deprecation decorator. Could you please add it? There are a couple of examples for it peppered through the modules.


@class_or_instance
def service_request_async(self, service, params, page_size=None, page=None, use_json=False, **kwargs):
def service_request_async(self, service, params, pagesize=None, page=None, use_json=False, **kwargs):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use a deprecation rename argument for this

@snbianco snbianco force-pushed the ASB-25227-catalogs-page-size branch from e1be3fe to 126ea05 Compare July 11, 2024 02:01
@bsipocz bsipocz added the mast label Jul 12, 2024
@bsipocz bsipocz added this to the v0.4.8 milestone Jul 12, 2024

@bsipocz bsipocz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@bsipocz bsipocz merged commit e19a779 into astropy:main Jul 14, 2024
@snbianco snbianco deleted the ASB-25227-catalogs-page-size branch July 17, 2024 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants