Cleanup/refactoring for 8.0

[dgw]

Originally posted by @half-duplex in #2062 (comment)

• sopel/formatting.py TODO (formatting: colors is now an Enum #2122)
• conftest.py 2x "Python 3.3" comments (conftest: remove hacks/shims for running tests on Python 3.3 #2123)
• sopel/plugins/rules.py has a 2.7 TODO (plugins.rules.URLCallback: use yield from expression #2124)
• [ ] sopel/irc/backends.py SSLAsynchatBackend.handle_connect NAK; replaced by asyncio backend

[dgw]

Got PRs for the first three. I'd have to study more about that SSLAsynchatBackend one, if it's even worth doing before @Exirel starts on that planned rewrite using asyncio instead of asynchat.

[Exirel]

Not sure it's worth it indeed.

On the other hand, I'm still pondering if using asyncio vs using a threading approach wouldn't be better anyway, since Sopel does not take advantage of asyncio in any way (there is no coroutine/async/await, and it won't be possible to use them any time soon).

[dgw]

Whether threading or asyncio, it's probably best to refactor Soon™. Looks like the current asyncore/asynchat backend is going to trigger DeprecationWarning in Python 3.10: python/cpython#26904

Honestly we should be surprised (and grateful) that it took this long (almost 5 years since 3.6.0 final).

[Exirel]

All done from this list.

[dgw]

All done from this list.

Is the backend one "done" by virtue of that class being replaced? 😁

[Exirel]

Absolutely. 😁
I don't know if there are other items to consider, so I didn't close the issue.

[dgw]

Consider? Yes. I found some stuff to triage by searching the whole repo for "todo". Good man realizing there could be more stuff that wasn't included in the list here. 😁

Py2 and cleanup/follow-through categories are the ones I'd consider important to either complete or update comments with a revised plan to handle in 8.1/9. Some of these items may require not-insignificant refactoring, which is why I included "update comments with a revised plan" as an option.

Future planning category would be nice to think about before we cut a new major version, but aren't required.

Misc. is obviously the least important. I threw things in there when I didn't necessarily understand what's being asked, or think it'll take under 15 mins to address.

Py2-is-dead stuff (fixed by #2326)

sopel/sopel/modules/currency.py

Lines 147 to 148 in b9b827a

	# TODO: Use this instead after dropping Python 2 support
	# amount, base, _, *targets = query.split()

sopel/sopel/plugins/handlers.py

Line 61 in 8b463a0

# TODO: imp is deprecated, to be removed when py2 support is dropped

sopel/sopel/plugins/handlers.py

Line 462 in 8b463a0

# TODO: switch to ``importlib`` when Python2 support is dropped.

sopel/sopel/tests/mocks.py

Line 121 in 5c4f56e

# TODO: `blocking` method args below should be made kwarg-ONLY in py3

Other cleanup/follow-through stuff (fixed by #2329 and #2330)

sopel/sopel/plugin.py

Lines 1407 to 1408 in 5c4f56e

	# old-style flags
	# TODO: to be removed in Sopel 8.0

sopel/sopel/modules/currency.py

Lines 75 to 79 in b9b827a

	elif config.currency.fixer_io_key is not None:
	# Must be unset or it will override the chosen fiat_provider
	# TODO: this is temporary for Sopel 7.x; see below
	print('Chosen provider is {}; clearing Fixer.io API key.'.format(config.currency.fiat_provider))
	config.currency.fixer_io_key = None

sopel/sopel/modules/currency.py

Lines 221 to 225 in b9b827a

	# Warning about future behavior change
	LOGGER.warning(
	'fiat_provider is set to %s, but Fixer.io API key is present; '
	'using Fixer anyway. In Sopel 8, fiat_provider will take precedence.',
	bot.config.currency.fiat_provider)

sopel/test/plugins/test_plugins_rules.py

Lines 1921 to 1923 in 5c4f56e

	def test_command_from_callable_regex_pattern(mockbot):
	# TODO: this test must FAIL for Sopel 8.0
	# Command name as regex pattern will be removed in Sopel 8.0

sopel/test/plugins/test_plugins_rules.py

Lines 2239 to 2241 in 5c4f56e

	def test_nick_command_from_callable_regex_pattern(mockbot):
	# TODO: this test must FAIL for Sopel 8.0
	# Command name as regex pattern will be removed in Sopel 8.0

sopel/test/plugins/test_plugins_rules.py

Lines 2444 to 2446 in 5c4f56e

	def test_action_command_from_callable_regex_pattern(mockbot):
	# TODO: this test must FAIL for Sopel 8.0
	# Command name as regex pattern will be removed in Sopel 8.0

Future planning stuff

sopel/sopel/irc/__init__.py

Lines 103 to 107 in 5c4f56e

	@property
	def config(self) -> Config:
	"""The :class:`sopel.config.Config` for the current Sopel instance."""
	# TODO: Deprecate config, replaced by settings
	return self.settings

sopel/sopel/modules/admin.py

Lines 425 to 426 in 5c4f56e

	# Fallback to guessing if secret, for backward compatiblity
	# TODO: consider a deprecation warning when loading settings

sopel/sopel/tools/web.py

Lines 135 to 136 in 5c4f56e

	# TODO deprecated?
	return html.unescape(text)

sopel/sopel/tools/web.py

Lines 151 to 152 in 5c4f56e

	# TODO deprecated?
	return urllib.parse.quote(str(string), safe)

sopel/sopel/tools/web.py

Lines 166 to 167 in 5c4f56e

	# TODO deprecated?
	return urllib.parse.unquote(string)

sopel/sopel/tools/web.py

Lines 200 to 202 in 5c4f56e

	# direct shortcut kept for backward compatibility reasons
	# TODO consider removing this
	urlencode = urllib.parse.urlencode

Misc., idk-might-be-easy stuff

sopel/sopel/tests/pytest_plugin.py

Line 90 in b9b827a

# TODO enable message tags

sopel/sopel/modules/tell.py

Lines 68 to 69 in b9b827a

	except ValueError:
	continue # TODO: Add warning log about malformed reminder

[dgw]

AFAIK this is actually done. If we missed anything, it'll get caught Eventually™ as we continue rewriting and extending stuff.