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

scala 3 api batch 2 #31

Merged
merged 4 commits into from
Jul 28, 2020
Merged

scala 3 api batch 2 #31

merged 4 commits into from
Jul 28, 2020

Conversation

Gryfit
Copy link

@Gryfit Gryfit commented Jul 16, 2020

to be clear enums will be added later.

@@ -5,14 +5,38 @@ import scala.language.implicitConversions
import scala.reflect.ClassTag

object PythonUnion {
def convert[X <: py.Any, Y <: py.Any, A <: PythonType[X], B <: PythonType[Y]](u: A | B)(implicit ev1: ClassTag[A], ev2: ClassTag[B]): py.|[X, Y] =

def fromPyhtonTypeAndScalaTypeUnion[X <: py.Any, A <: PythonType[X], B <: Any](u: A | B)(implicit ev1: ClassTag[A], ev2: ClassTag[B]): py.|[B, X] =
Copy link
Author

Choose a reason for hiding this comment

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

This cannot be implicit conversion without some clever type bound on B because it collides with the one beneath fromPyhtonTypesUnion

Base automatically changed from scala3API to develop July 27, 2020 10:16
@Gryfit
Copy link
Author

Gryfit commented Jul 27, 2020

cleared up git history

println(s"x_test shape: ${x_test1.shape}")
val xTrain1 = sequence.padSequences(xTrain, maxLen = Some(maxlen)).astype(np.float32)
val xTest1 = sequence.padSequences(xTest, maxLen = Some(maxlen)).astype(np.float32)
val yTrain1 = yTrain.astype(np.float32)
Copy link

@piotrmwojcik piotrmwojcik Jul 27, 2020

Choose a reason for hiding this comment

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

Why are you casting interger ({0, 1}?) value to a float?

Copy link
Author

@Gryfit Gryfit Jul 28, 2020

Choose a reason for hiding this comment

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

There are no generic types implemented in lower layer (scala 2) for now, we picked Float as a universal type because it was convenient. (created issue to add generic types VirtuslabRnD/scalapy#3 )
But we can use Numeric in this layer and cast it to Float when passing to scala 2 layer.

Copy link
Author

Choose a reason for hiding this comment

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

Made an issue, will take care of that later, It causes build problems

import preprocessing.Preprocessing
import models.Models
import datasets.Datasets
import backend.Backend
class Keras private[api] (val underlying: PyKeras) extends PythonType[PyKeras] with PythonModule {

Choose a reason for hiding this comment

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

This comment is for whole PR as well as for other batches -- I'm strongly convinced that exposing public api without a line of comment does not make users' lives easier.

Copy link
Author

@Gryfit Gryfit Jul 28, 2020

Choose a reason for hiding this comment

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

well thats true, although documentation can be found on tensorflow website.
Anyway we had somewhat inconclusive discussion about this. Lets make an issue #38 :D and don't worry for now.

Copy link

@PawelLipski PawelLipski left a comment

Choose a reason for hiding this comment

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

Minor comments, approving

@Gryfit Gryfit merged commit b08db86 into develop Jul 28, 2020
@Gryfit Gryfit deleted the scala3API-2 branch July 28, 2020 15:29
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.

3 participants