Skip to content

Conversation

@Vedant936-ai
Copy link

@Vedant936-ai Vedant936-ai commented Dec 3, 2025

Description

What is this PR?

This pull request updates the atlas listing functions so that they return atlas names in alphabetical order. This improves clarity, user experience, and consistency across the API. The change addresses Issue #612.

Why is this PR needed?

Currently, the functions return atlas names based on the chronological order in which they were downloaded. This ordering can be confusing for users and makes it harder to locate specific atlases. Alphabetical ordering provides a predictable and user-friendly structure.

What does this PR do?

  • Introduces alphabetical sorting for both downloaded atlases and available atlases from the BrainGlobe registry.
  • Simplifies and improves the readability of the atlas listing logic.
  • Ensures consistent ordering across both sources of atlas names.

References

This PR directly addresses and resolves Issue #612.

How has this PR been tested?

  • The modified functions were executed locally to confirm that the output now consistently appears in alphabetical order.
  • Both downloaded atlas lists and remote registry atlas lists were verified.
  • Checked that no other behavior or functionality was affected.

Is this a breaking change?

This is not a breaking change. It only modifies the order in which results are returned, without altering the underlying data structures or API behavior.

Does this PR require documentation updates?

No documentation updates are required as the functionality of the API remains unchanged. Only the ordering of results is improved.


Checklist

  • Code has been tested locally
  • Tests for new behavior can be added if maintainers request
  • No documentation changes required
  • Code follows the project's formatting and contribution guidelines

Vedant Deshmukh and others added 2 commits December 3, 2025 20:16
…inglobe#612 where updated the atlas listing functions to return results in alphabetical order instead of chronological downloaded order
@Vedant936-ai Vedant936-ai reopened this Dec 3, 2025
@Vedant936-ai
Copy link
Author

Hi! This is my first contribution to Brainglobe 😊

The CI is waiting for workflow approval since this is a first-time contribution.
Please feel free to approve it whenever convenient — I’m happy to make any changes if needed.
Thank you!

@adamltyson
Copy link
Member

Thanks @Vedant936-ai. Someone will review it when there is time. In the meantime, could I ask why the docstrings were removed?

@Vedant936-ai
Copy link
Author

Thanks @Vedant936-ai. Someone will review it when there is time. In the meantime, could I ask why the docstrings were removed?

Oh , about that the docstring got removed because the pre-commit auto-fix changed it when I tried to fix the D200 lint error. I will insert it again

@adamltyson
Copy link
Member

adamltyson commented Dec 18, 2025

Hi @Vedant936-ai, are you able to get back to this? No worries either way.

@Vedant936-ai
Copy link
Author

Hi @Vedant936-ai, are you able to get back this? No worries either way.

Hey @adamltyson . Yes the docstrings are added . Actually it was added way before but conflicts were yet to be resolved . Now I all the conflicts are resolved. And it's proper now , you may check it .

Copy link
Member

@alessandrofelder alessandrofelder left a comment

Choose a reason for hiding this comment

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

Thanks @Vedant936-ai

Code changes look reasonable to me, and I did some manual testing successfully.

It would be good to take this opportunity to write some tests in https://github.com/brainglobe/brainglobe-atlasapi/blob/main/tests/atlasapi/test_list_atlases.py that check the alphabetical order is returned by each of the functions we've modified here.

Let us know if you'd be willing to add this to the PR (I will be happy either way) - thanks!

@Vedant936-ai
Copy link
Author

Thanks @Vedant936-ai

Code changes look reasonable to me, and I did some manual testing successfully.

It would be good to take this opportunity to write some tests in https://github.com/brainglobe/brainglobe-atlasapi/blob/main/tests/atlasapi/test_list_atlases.py that check the alphabetical order is returned by each of the functions we've modified here.

Let us know if you'd be willing to add this to the PR (I will be happy either way) - thanks!

Hi @alessandrofelder Sure , I will add the tests for alphabetical order fn in the file and will commit to this PR only . Thankyou

@Vedant936-ai Vedant936-ai changed the title updated the atlas listing functions to return results in alphabetical order instead of chronological downloaded order #612 feat: Sort atlas listings alphabetically and add corresponding tests (#612) Dec 23, 2025
@Vedant936-ai
Copy link
Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants