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

Delay transforming priority_order into ndarray #969

Closed

Conversation

junholee6a
Copy link
Contributor

Issue: #722

In the changed code, we had a mypy error because numpy ndarrays are not compatible with random.Random.shuffle() (expected argument type is MutableSequence[Any])

We fix this by first instantiating priority_order as a list, then shuffling it, then creating an ndarray from it afterwards.

lizlouise1335 and others added 8 commits July 6, 2023 14:54
* Fixed F1Score Import

* Linted example file with Black Linter
* update

* renamed var and removed from for loops

* refactored var
…apitalone#954)

A method signature that uses *args: Any, **kwargs: Any is compatible
with any set of arguments in mypy, despite being an LSP violation. This
lets us assert that subclasses of BaseDataProcessor should have some
process() method with an arbitrary signature.

We also add to the return type of BaseDataPreprocessor so that it is
inclusive of all of its subclasses.

Co-authored-by: JGSweets <[email protected]>
Inside the BaseDataProcessor class definition, references to
__subclasses are automatically replaced with
_BaseDataProcessor__subclasses. This remains the case even in static
methods _register_subclass() and get_class(). Same with BaseModel and
its __subclasses field. So we do not have to write out the full name
mangled identifiers inside the class definitions.

Also, mypy doesn't seem to be able to handle the return type of
BaseDataProcessor.get_class() being a typevar, so that was changed to
type[BaseDataProcessor]. This does not affect the functionality of
get_class() since it always returns a subclass of BaseDataProcessor.
The mypy errors addressed here occur because variables label_mapping
(in CharPreprocessor), unstructured_labels, and unstructured_label_set
(in StructCharPreprocessor.process()) have optional types when they're
used. This is fixed by checking that they are not None prior to the
operation, which mypy recognizes as removing the None type from them.

This should have no effect on functionality because we are already
checking that labels is not None, and the variables above all depend on
labels such that they are None only if labels is None.
capitalone#965)

* Changed release option to only release branches named \'release/<version-tag>\'.

* Reverted types
In the changed code, we had a mypy error because numpy ndarrays are not
compatible with random.Random.shuffle() (expected argument type is
MutableSequence[Any])

We fix this by first instantiating priority_order as a list, then
shuffling it, then creating an ndarray from it afterwards.
@taylorfturner
Copy link
Contributor

update branch

@junholee6a
Copy link
Contributor Author

update branch

Will update this branch after PR #968 is no longer open

tyfarnan
tyfarnan previously approved these changes Jul 27, 2023
ksneab7
ksneab7 previously approved these changes Aug 7, 2023
Copy link
Contributor

@taylorfturner taylorfturner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking for 0.10.3 release. will unblock post-deployment

@taylorfturner taylorfturner changed the base branch from old-dev to dev August 9, 2023 15:37
@taylorfturner taylorfturner dismissed stale reviews from ksneab7 and tyfarnan August 9, 2023 15:37

The base branch was changed.

@taylorfturner
Copy link
Contributor

@junholee6a update and resolve conflict on requirements.txt for this PR

auto-merge was automatically disabled September 17, 2023 22:33

Head branch was pushed to by a user without write access

@junholee6a
Copy link
Contributor Author

Conflict was fixed and up to date

@taylorfturner taylorfturner deleted the branch capitalone:old-old-dev September 26, 2023 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
static_typing mypy static typing issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants