Skip to content

Make all internal imports relative#386

Merged
gijzelaerr merged 1 commit into
gijzelaerr:masterfrom
vmsh0:relative-imports
Sep 22, 2022
Merged

Make all internal imports relative#386
gijzelaerr merged 1 commit into
gijzelaerr:masterfrom
vmsh0:relative-imports

Conversation

@vmsh0
Copy link
Copy Markdown
Contributor

@vmsh0 vmsh0 commented Aug 23, 2022

Not sure whether this is welcome, but we're using is as an internal patch and I thought it would be nice to offer it here.

This basically allows more freedom installing and moving the snap7 module around the filesystem for peculiar setups, as far as I know it's a pretty common practice for Python libraries.

@vmsh0
Copy link
Copy Markdown
Contributor Author

vmsh0 commented Aug 23, 2022

Not sure why some tests are failing, is it due to my changes? Will check more later.

@swamper123
Copy link
Copy Markdown
Contributor

Relative imports can be handy, as long as you know what you do. I made the experience, that in more nested projects, many people fall into an ImportError pit (but that is another story).

Anyway - back to the topic: In our case I am not so happy, because of types. types is a buitin module itself, but also a module in our repo. If we want to use relative imports, a refactoring has to happen to differ both things better for readability (and maybe future conflicts) reasons. Other opinions are welcomed :)

@vmsh0
Copy link
Copy Markdown
Contributor Author

vmsh0 commented Aug 24, 2022

Anyway - back to the topic: In our case I am not so happy, because of types. types is a buitin module itself, but also a module in our repo. If we want to use relative imports, a refactoring has to happen to differ both things better for readability (and maybe future conflicts) reasons. Other opinions are welcomed :)

Very good point, but as long as the built-in types is not used in the project I don't see it as a terrible thing. (at least compared to what it is already)

Comment thread snap7/client.py Outdated
@vmsh0 vmsh0 force-pushed the relative-imports branch 2 times, most recently from 3871047 to 5ce5b4b Compare September 21, 2022 14:14
this allows more freedom installing and moving the snap7 module around
the filesystem for peculiar setups.
@vmsh0
Copy link
Copy Markdown
Contributor Author

vmsh0 commented Sep 21, 2022

I've changed all the types imports to drop the prefix and just explicitly import the names.

@vmsh0 vmsh0 requested a review from gijzelaerr September 21, 2022 14:43
@gijzelaerr gijzelaerr merged commit 076dbee into gijzelaerr:master Sep 22, 2022
@gijzelaerr gijzelaerr added this to the 1.3 milestone Sep 22, 2022
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