Skip to content

add public modules to use direct import first stage like 'import astr… - #17587

Closed
celik-muhammed wants to merge 1 commit into
astropy:mainfrom
celik-muhammed:subpackage-bug-fix
Closed

celik-muhammed wants to merge 1 commit into
astropy:mainfrom
celik-muhammed:subpackage-bug-fix

Conversation

@celik-muhammed

@celik-muhammed celik-muhammed commented Dec 28, 2024 •

Copy link
Copy Markdown

… 'import astropy.stats as st'

Description

This pull request is to address ...

Fixes #

  • By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

@github-actions

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Welcome to Astropy 👋 and congratulations on your first pull request! 🎉

A project member will respond to you as soon as possible; in the meantime, please have a look over the Checklist for Contributed Code and make sure you've addressed as many of the questions there as possible.

If you feel that this pull request has not been responded to in a timely manner, please send a message directly to the development mailing list. If the issue is urgent or sensitive in nature (e.g., a security vulnerability) please send an e-mail directly to the private e-mail [email protected].

@neutrinoceros

Copy link
Copy Markdown
Contributor

Hi ! Could you explain your motivation for this change ? Unfortunately, exposing more public API paths comes with a very high maintenance cost so I must say it's unlikely we'd accept it. Maybe we could offer a better solution if you told us what the problem was ?

@celik-muhammed

Copy link
Copy Markdown
Author

Thanks for explanation,
for new users for this lib like me try to use sample stats lib via 'import astropy.stats as st'.
documentation also define use 'from astropy import stats' but ı think this import style just suggest.

@pllim pllim 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.

Hello. Unfortunately I do not understand what this is actually trying to solve. Even without this patch, import astropy.stats as st already works.

@pllim pllim added this to the v7.1.0 milestone Dec 30, 2024
@pllim pllim added the API change PRs and issues that change an existing API, possibly requiring a deprecation period label Dec 30, 2024
@celik-muhammed

Copy link
Copy Markdown
Author

Yes, it works without this patch; I checked it.

In my specific scenario, where the internet is not available for some time,
I need the stats library for searching. I have an older version 6.1.6 installed, so I tried:

import astropy
dir(astropy)

The stats module is not present, so I assume it was added in a later version.
Currently, I haven’t checked other versions or the online documentation.

I want to emphasize that when working offline, having the available submodules listed in the result of the dir function or including them in the docstring would be much more useful.

@pllim

pllim commented Dec 31, 2024

Copy link
Copy Markdown
Member

I see. Thank you for bringing this to our attention, I agree that the top level namespace could be cleaned up. I have opened #17589 as a follow-up issue for discussion and we can always revisit this PR in the future once consensus is reached. Thanks again!

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

Labels

API change PRs and issues that change an existing API, possibly requiring a deprecation period

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants