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

Pin numpy version in dev requirements, only register type transformer if installed #2485

Merged
merged 14 commits into from
Jun 18, 2024

Conversation

cosmicBboy
Copy link
Contributor

@cosmicBboy cosmicBboy commented Jun 17, 2024

This is a follow up PR on #2483.

This PR updates flytekit's imports of numpy:

  • Only register the numpy ndarray type transformer if numpy is installed
  • Only define the numpy supported types in the Schema and StructuredDataset types if numpy is installed
  • Pin numpy < 2 in ci builds involving numpy

cosmicBboy and others added 6 commits June 17, 2024 13:43
Signed-off-by: Niels Bantilan <[email protected]>
Signed-off-by: Niels Bantilan <[email protected]>
Signed-off-by: Niels Bantilan <[email protected]>
Signed-off-by: Niels Bantilan <[email protected]>
Copy link

codecov bot commented Jun 18, 2024

Codecov Report

Attention: Patch coverage is 55.55556% with 4 lines in your changes missing coverage. Please review.

Project coverage is 71.92%. Comparing base (8562fd9) to head (9281870).
Report is 9 commits behind head on master.

Files Patch % Lines
flytekit/types/numpy/__init__.py 55.55% 3 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2485      +/-   ##
==========================================
- Coverage   73.97%   71.92%   -2.06%     
==========================================
  Files         250      182      -68     
  Lines       21592    18559    -3033     
  Branches     3644     3653       +9     
==========================================
- Hits        15973    13348    -2625     
+ Misses       4973     4567     -406     
+ Partials      646      644       -2     

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

Signed-off-by: Niels Bantilan <[email protected]>
Signed-off-by: Niels Bantilan <[email protected]>
Signed-off-by: Niels Bantilan <[email protected]>
Signed-off-by: Niels Bantilan <[email protected]>
@cosmicBboy cosmicBboy merged commit 7265c9e into master Jun 18, 2024
45 of 48 checks passed
bgedik pushed a commit to bgedik/flytekit that referenced this pull request Jul 3, 2024
… if installed (flyteorg#2485)

* remove numpy dep, conditionally register numpy type transformer

Signed-off-by: Niels Bantilan <[email protected]>

* pin numpy version <2 in dev requirements

Signed-off-by: Niels Bantilan <[email protected]>

* update numpy dep in build-with-pandas

Signed-off-by: Niels Bantilan <[email protected]>

* numpy matrix

Signed-off-by: Niels Bantilan <[email protected]>

* pin numpy<2 in onnx plugins

Signed-off-by: Niels Bantilan <[email protected]>

* lint

Signed-off-by: Niels Bantilan <[email protected]>

* pin numpy in ci

Signed-off-by: Niels Bantilan <[email protected]>

* Update pythonbuild.yml

* remove numpy import from top-level of types.numpy.__init__

Signed-off-by: Niels Bantilan <[email protected]>

* lint

Signed-off-by: Niels Bantilan <[email protected]>

* revert changes to schema and structured dataset since these are lazily loaded anyway

Signed-off-by: Niels Bantilan <[email protected]>

* debug

Signed-off-by: Niels Bantilan <[email protected]>

* numpy version pin on sqlalchemy

Signed-off-by: Niels Bantilan <[email protected]>

* lint

Signed-off-by: Niels Bantilan <[email protected]>

---------

Signed-off-by: Niels Bantilan <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Signed-off-by: bugra.gedik <[email protected]>
fiedlerNr9 pushed a commit that referenced this pull request Jul 25, 2024
… if installed (#2485)

* remove numpy dep, conditionally register numpy type transformer

Signed-off-by: Niels Bantilan <[email protected]>

* pin numpy version <2 in dev requirements

Signed-off-by: Niels Bantilan <[email protected]>

* update numpy dep in build-with-pandas

Signed-off-by: Niels Bantilan <[email protected]>

* numpy matrix

Signed-off-by: Niels Bantilan <[email protected]>

* pin numpy<2 in onnx plugins

Signed-off-by: Niels Bantilan <[email protected]>

* lint

Signed-off-by: Niels Bantilan <[email protected]>

* pin numpy in ci

Signed-off-by: Niels Bantilan <[email protected]>

* Update pythonbuild.yml

* remove numpy import from top-level of types.numpy.__init__

Signed-off-by: Niels Bantilan <[email protected]>

* lint

Signed-off-by: Niels Bantilan <[email protected]>

* revert changes to schema and structured dataset since these are lazily loaded anyway

Signed-off-by: Niels Bantilan <[email protected]>

* debug

Signed-off-by: Niels Bantilan <[email protected]>

* numpy version pin on sqlalchemy

Signed-off-by: Niels Bantilan <[email protected]>

* lint

Signed-off-by: Niels Bantilan <[email protected]>

---------

Signed-off-by: Niels Bantilan <[email protected]>
Co-authored-by: Eduardo Apolinario <[email protected]>
Signed-off-by: Jan Fiedler <[email protected]>
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