[PATCH] D139370: [mlgo] Dependency-free training mode logger

[Jacob Hegna via Phabricator via llvm-commits]

jacobhegna added inline comments.


================
Comment at: llvm/lib/Analysis/models/log_reader.py:34
+  @classmethod
+  def from_dict(d: dict):
+    name = d['name']
----------------
mtrofin wrote:
> jacobhegna wrote:
> > this won't work, no? if it's a `classmethod` the first argument could be `cls`, and the point is you could return `cls(name=...)` instead of `TensorSpec(name=...)`, it's essentially the same but follows the convention I see people using for factory methods in python
> (sorry, went to fast over it)
> 
> what does it give us, though, compared to the @staticmethod way?
in this case, basically nothing 😊 (that's why it was a nit)

in this case, it's just a stylistic choice: I tend to see people use `@classmethod` for factory-style methods, and `@staticmethod` for methods that don't return an instance of the type. it's not that important, though


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139370/new/

https://reviews.llvm.org/D139370