caronc / apprise

Apprise - Push Notifications that work with just about every platform!
https://hub.docker.com/r/caronc/apprise
BSD 2-Clause "Simplified" License
10.9k stars 384 forks source link

Python Module Naming & Namespacing Harmonization #1119

Closed caronc closed 2 months ago

caronc commented 2 months ago

Description:

Related issue (if applicable): cpython/117860

The short of it is:The Python language has a lot of ambiguity when the name you give your Python file aligns with a class, function or variable within it.

Apprise for years was built on structuring it's files as:

Up until Python v3.11 this wasn't an issue. However due to underlining changes to unittest, testing from 3rd party applications no longer worked and there was no backwards compatibility.

The cpython/117860 ticket revealed that not only is there ambiguity with the unittest, but it extends in import calls as well which Apprise was subject to bizarre issues as well, but they were just never brought forth over the past 4 years because the documentation didn't identify/illustrate it and no one appeared to have any issues. None that were reported here anyway.

This is a major change with likely breaking side-effects. It's still a WIP, but the goal is to find ways of cleaning up the (Python) file namespacing to work around this limitation of Python.

Testing

Anyone can help test this source code as follows:

# Create a virtual environment to work in as follows:
python3 -m venv apprise

# Change into our new directory
cd apprise

# Activate our virtual environment
source bin/activate

# Install the branch
pip install git+https://github.com/caronc/apprise.git@python-fname-refactor

# Test out the changes with the following command:
apprise -t "Test Title" -b "Test Message" \
  <apprise url related to ticket>
codecov-commenter commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.28%. Comparing base (08cb018) to head (ed595e1).

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1119 +/- ## ======================================= Coverage 99.28% 99.28% ======================================= Files 139 139 Lines 18056 18066 +10 Branches 3689 3689 ======================================= + Hits 17926 17936 +10 Misses 121 121 Partials 9 9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

mattpackwood commented 2 months ago

I tested and had no issues with the following:

caronc commented 2 months ago

@mattpackwood: I'm not concerned about the notifications not working. There are more then enough unit tests to check that.

The concerns will be for developers who import the library into their own environment. I THINK the naming "as is" should cause minimal issues, but more importantly, it will be future proof for newer versions of Python