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

Fix bug that occurs with dataclass_json 0.62 #493

Merged
merged 2 commits into from
Nov 13, 2023

Conversation

zigaLuksic
Copy link
Collaborator

The issue happens when someone calls the to_dict method on the collection.
Sadly this is used in our code a lot 🙃 would you consider this worthy of a release?

@zigaLuksic zigaLuksic requested a review from mlubej November 13, 2023 08:01
@zigaLuksic
Copy link
Collaborator Author

i guess a release makes sense also from the point that eo-grow tests get fixed. Otherwise we have to skip the byoc pipeline tests for a bit.

Copy link
Contributor

@mlubej mlubej left a comment

Choose a reason for hiding this comment

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

A minor release seems completely suitable to me.

Comment on lines 161 to 165
if len(bbox) == 4:
min_x, min_y, max_x, max_y = cast(Tuple[float, float, float, float], bbox)
min_x, min_y, max_x, max_y = bbox
else:
(min_x, min_y), (max_x, max_y) = cast(Tuple[Tuple[float, float], Tuple[float, float]], bbox)
(min_x, min_y), (max_x, max_y) = bbox
return float(min_x), float(min_y), float(max_x), float(max_y)
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to limit the various options how to define a bbox? it seems like a similar situation as in case of features in eolearn. But this is a separate issue, so this is just a comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we already did quite a bit
the problem with that is that it is fairly code-breaking, so it's a sin of the past that we're slowly correcting

@zigaLuksic zigaLuksic merged commit 6f8cbb4 into develop Nov 13, 2023
7 checks passed
zigaLuksic added a commit that referenced this pull request Nov 13, 2023
* Enable Github actions to trigger internal Gitlab CI (#490)

* update actions and gitlab CI

* update project path

* use trigger API to build internal docker instead

* test trigger on branch push

* fix github action

* test why trigger workflow not starting

* fix branch name

* remove step in trigger job

* remove test branch

* move mirror to a separate workflow, update pipeline running rules

* Revert "move mirror to a separate workflow, update pipeline running rules"

This reverts commit 92b4b04.

---------

Co-authored-by: Matic Lubej <[email protected]>

* deprecate deprecation wrappers

* switch to type_extensions deprecations

* add comments

* Fix bug that occurs with dataclass_json 0.62 (#493)

* remove "undefined" catch-all flag from a field

* mypy fixes

* fix wrong import

* remove redundant import

---------

Co-authored-by: Matic Lubej <[email protected]>
Co-authored-by: Matic Lubej <[email protected]>
@zigaLuksic zigaLuksic deleted the fix-dataclass-json-bug branch November 14, 2023 09:34
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.

2 participants