Skip to content

Conversation

@kevinmessiaen
Copy link
Member

@kevinmessiaen kevinmessiaen commented Jun 26, 2023

Description

  • Fixed recursive issues
  • Export tests/slicing/transformation
  • Fix issue with parameters

Related Issue

Type of Change

  • 📚 Examples / docs / tutorials / dependencies update
  • 🔧 Bug fix (non-breaking change which fixes an issue)
  • 🥂 Improvement (non-breaking change which improves an existing feature)
  • 🚀 New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to change)
  • 🔐 Security fix

Checklist

  • I've read the CODE_OF_CONDUCT.md document.
  • I've read the CONTRIBUTING.md guide.
  • I've updated the code style using make codestyle.
  • I've written tests for all new methods and classes that I created.
  • I've written the docstring in Google format for all the methods and classes that I used.

@linear
Copy link

linear bot commented Jun 26, 2023

GSK-1243 Project export doesn't work

  • Follow german credit scorring notebook
  • after test suite upload export the project

giskard-diagnose-2_0_0b2-20230606_220437_713668.tar.gz

@kevinmessiaen kevinmessiaen marked this pull request as ready for review June 27, 2023 06:31
@kevinmessiaen kevinmessiaen marked this pull request as draft July 27, 2023 03:10
@Inokinoki
Copy link
Member

Is this still WIP?

I could review if it's ready and up-to-date

@kevinmessiaen
Copy link
Member Author

@Inokinoki This PR needs to be updated with main, the export might be broken due to modification of database schema. It was initially on hold due to ongoing refactor of artifacts but we can merge it.

@kevinmessiaen kevinmessiaen marked this pull request as ready for review September 5, 2023 06:50
@kevinmessiaen
Copy link
Member Author

@Inokinoki I've updated the branch and the export seems to be working with latest modification

@Inokinoki
Copy link
Member

Inokinoki commented Sep 11, 2023

LGTM
Tested locally, working perfectly!

Could the 5 code smells in SonarLint be fixed as well?

In addition, should we display something like "export failed" when getting 500 in frontend side? I can add it if you do not have time

@Inokinoki
Copy link
Member

Inokinoki commented Sep 13, 2023

I tried to improve the code smell stuff from SonarLint. But I am not sure how to implement the comparison in the remaining one.
Could @kevinmessiaen please review them?

@kevinmessiaen
Copy link
Member Author

I tried to improve the code smell stuff from SonarLint. But I am not sure how to implement the comparison in the remaining one. Could @kevinmessiaen please review them?

Sure I fixed the error. In this case we had to override it to make sure two instance of Callable with same uuid but a different class (eg. SlicingFunction and TransformationFunction) are not interpreted has equals.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@Inokinoki
Copy link
Member

LGTM

@Inokinoki Inokinoki merged commit ab5f014 into main Sep 14, 2023
@Inokinoki Inokinoki deleted the feature/gsk-1243-project-export-doesnt-work branch September 14, 2023 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Development

Successfully merging this pull request may close these issues.

5 participants