Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python Module Naming & Namespacing Harmonization #1119

Merged
merged 3 commits into from
Apr 27, 2024
Merged

Conversation

caronc
Copy link
Owner

@caronc caronc commented Apr 26, 2024

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:

  • /notify/ClassName.py with a class ClassName() inside of it.

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
Copy link

codecov-commenter commented Apr 26, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

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

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

Additional details and impacted files
@@           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           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mattpackwood
Copy link
Contributor

I tested and had no issues with the following:

  • AWS SNS
  • Discord
  • IFTTT
  • Join
  • LaMetric - Local only
  • Line
  • Notifiarr
  • Ntfy
  • prowl
  • Pushbullet
  • Push (Techulus)
  • Pushed
  • PushSafer
  • Pushover
  • Reddit
  • SimplePush
  • Telegram
  • Twist

@caronc
Copy link
Owner Author

caronc commented Apr 27, 2024

@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

@caronc caronc merged commit b8da133 into master Apr 27, 2024
13 checks passed
@caronc caronc deleted the python-fname-refactor branch May 28, 2024 00:50
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.

None yet

3 participants