Skip to content

Conversation

@utnapischtim
Copy link
Contributor

  • nicer work, but useless, because of following commit
  • feat: improve import usage

* the warning was not working as intended.

* since it is complicated programmatically to distinguish between
  'import idutils' and 'from idutils import XY' to write a warning
  only on one of the both cases and it is not the packages business
  to decide how the package which is using it will import functions
  from modules, the whole warning has been removed and the user can
  decide how it will be imported

* relying only on 'from idutils.module_a import func_xy' would mean that
  all all packages which are using this have to rewrite there code with
  not really any real benefit

* all those points lead to the removal of the warning. btw this behavior
  is the same now as it was with the warning.
@utnapischtim utnapischtim force-pushed the fix-implicit-import-warning branch from f429a37 to 010612b Compare December 2, 2025 21:05
Copy link
Contributor

@tmorrell tmorrell left a comment

Choose a reason for hiding this comment

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

I don't think this works if you're importing the module from outside of the idutils directory. If I install the fork with pip and run from the idutils directory

import idutils
print(dir(idutils))

I get all the functions. But if I move to another directory on my machine and run the same code I just get

['__doc__', '__file__', '__loader__', '__name__', '__package__', '__path__', '__spec__']

I think there is an location implied in the . import

Copy link
Contributor

@tmorrell tmorrell left a comment

Choose a reason for hiding this comment

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

I got confused by a separate bug on editable installs.

@utnapischtim utnapischtim moved this to To release 🤖 in vNext Dec 14, 2025
@utnapischtim
Copy link
Contributor Author

utnapischtim commented Dec 14, 2025

NOTE:
hold back for the moment.
reason:
There will be a bigger test running next week. to not interfere with it, it will be merged in January

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

Labels

None yet

Projects

Status: To release 🤖

Development

Successfully merging this pull request may close these issues.

3 participants