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

Convert control types constants into enums #10732

Closed
feerrenrut opened this issue Jan 30, 2020 · 9 comments · Fixed by #12510
Closed

Convert control types constants into enums #10732

feerrenrut opened this issue Jan 30, 2020 · 9 comments · Fixed by #12510
Assignees
Labels
component/speech deprecated/2022.1 Label used to track deprecations due for removal in the 2022.1 release
Milestone

Comments

@feerrenrut
Copy link
Contributor

feerrenrut commented Jan 30, 2020

This issue stemmed from a comment in #10703

As a follow up, do the same for the rest of the constants in control types, and ensure that all of NVDA uses the enum versions and officially deprecate (and give a date of removal for) the module level constants.

While doing this, add a deprecation comment to SpeechReason constants also.

Originally posted by @feerrenrut in #10703

@feerrenrut

This comment has been minimized.

@feerrenrut

This comment has been minimized.

@feerrenrut feerrenrut reopened this Mar 3, 2020
@feerrenrut feerrenrut modified the milestones: 2020.1, 2020.2 Mar 9, 2020
@LeonarddeR
Copy link
Collaborator

I think there are two implementation strategies:

  1. Define the enums and after that, do a setattr on the module to add the old constants
  2. Define a getattr at the module level that looks up the constants from the enums.

Performance wise, I'd do the former, but if you want to raise deprecation warnings, the latter is probably more effective.

Have you also considered constants in oleacc.py? I'd leave them as is.

@LeonarddeR
Copy link
Collaborator

An additional thought... May be we should advertise de labels using the enum itself?

class Role(enum.IntEnum):
	UNKNOWN = 0
	@property
	def label(self):
		return controlTypes.roleLabels[self]

@feerrenrut
Copy link
Contributor Author

Pushing this back to 2020.3

@feerrenrut
Copy link
Contributor Author

Given the possibility for breaking compatibility, I'm pushing this back to 2021.1

@feerrenrut feerrenrut removed this from the 2020.3 milestone Jul 30, 2020
@feerrenrut feerrenrut added the deprecated/2021.1 Label used to track deprecations due for removal in the 2021.1 release label Jul 30, 2020
@feerrenrut feerrenrut added this to the 2021.1 milestone Jul 30, 2020
@seanbudd seanbudd mentioned this issue Mar 5, 2021
18 tasks
@feerrenrut
Copy link
Contributor Author

Rather than do this all in one go, instead, similar to "Use enum for speech reason #10703" first we'll do this in a backwards compatible way so that add-on authors have a transitionary period. Later, probably 2022.1 we'll remove the deprecated aspects.

@seanbudd seanbudd added the deprecated/2022.1 Label used to track deprecations due for removal in the 2022.1 release label Mar 12, 2021
@seanbudd
Copy link
Member

@feerrenrut - would the backwards compatible work be part of deprecations/2021.1 or should I start on this separately?

@feerrenrut
Copy link
Contributor Author

@seanbudd the deprecation warnings don't necessarily need to be in this release. I think we should label this for 2022.1. The initial work to deprecate and provide a new alternative can be done after we get to beta for 2021.1.

@seanbudd seanbudd removed the deprecated/2021.1 Label used to track deprecations due for removal in the 2021.1 release label Mar 12, 2021
@seanbudd seanbudd self-assigned this Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/speech deprecated/2022.1 Label used to track deprecations due for removal in the 2022.1 release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants