search

REGnosys / rosetta-code-generators

Write code generators for any language, based on the rosetta DSL
Apache License 2.0
17 stars 33 forks source link

Python generator #204

Closed dcarrascosa00 closed 1 year ago

dcarrascosa00 commented 1 year ago

@SimonCockx all changes adapted

SimonCockx commented 1 year ago

@dcarrascosa00 Looking good! I'm off today, but I should be able to merge this on Wednesday. (what do you think @hugohills? You can already go ahead if you want)

SimonCockx commented 1 year ago

I just had a chat with Hugo, who tried to set up the code generation for the CDM using this PR. Two conclusions arose:

  1. Our infrastructure for supporting code generators is messy, to say the least. It's old and needs a proper clean-up. Our apologies for the unfriendly API we currently provide.
  2. The generated Python code looks great, but the interface that we depend on - AbstractExternalGenerator, which is implemented by the PythonCodeGenerator class - doesn't do what we expect it to do. (in fact, it doesn't do anything at all right now)

During the CDM build, for each code generator we make a single call per Rosetta file to the method AbstractExternalGenerator::generate. We expect it to return a Map<String, CharSequence> where each key represents a file path and each value represents the content of the file. That is - we expect a single call to it to produce all generated files at once for that Rosetta input file. It seems that you have currently implemented this functionality in the PythonFilesGeneratorTest class.

The generate method accepts three arguments:

  1. RosettaJavaPackages packages: deprecated - this is purely related to our Java code generator and should not be there. Please do not rely on it as it will be removed.
  2. List<RosettaRootElement> elements: These are the data types, functions, etc that are present in the file, and for which you can make calls to your appropriate specific generators, e.g., PythonEnumGenerator, PythonFunctionGenerator, etc. If you are interested in accessing the namespace of these elements, you can call element.getModel().getName() on any element in this list. In the future, we will pass this in directly to simplify this.
  3. String version: this is the version of the model. @dschwartznyc - you can count on this version to be the one you need.

Note that the keys of the output represent relative paths. The root of the generated code does not need to be known - choosing one is the responsibility of the consumer, i.e., the CDM build.

Could I ask you to move the functionality located in PythonFilesGeneratorTest into the PythonCodeGenerator class? If the AbstractExternalGenerator class doesn't provide you with the necessary hooks or arguments, I would be able to get a DSL patch out pretty quickly to support whatever you need - just let me know.

dschwartznyc commented 1 year ago

@SimonCockx @hugohills - we will take a look at migrating the functionality. Looking through the Scala implementation as an example, it doesn't seem like the generate function is called. Therefore, it would be really helpful to access more detailed documentation on the generator process (parse, generate, after generate, et al). Would that be possible?

Also, is there a concern that when parsing Rosetta files defining CDM v 3.3.2 and 3.5.0 we see the following error?

0 [main] ERROR osetta.scoping.RosettaScopeProvider - Error scoping rosetta - "null" see debug logging for full trace Additionally, is it concerning that when creating Scala from v 3.5.0, an exception is thrown when after generate is called on the model "cdm.product.template?"

SimonCockx commented 1 year ago

@SimonCockx @hugohills - we will take a look at migrating the functionality. Looking through the Scala implementation as an example, it doesn't seem like the generate function is called. Therefore, it would be really helpful to access more detailed documentation on the generator process (parse, generate, after generate, et al). Would that be possible?

Also, is there a concern that when parsing Rosetta files defining CDM v 3.3.2 and 3.5.0 we see the following error?

0 [main] ERROR osetta.scoping.RosettaScopeProvider - Error scoping rosetta - "null" see debug logging for full trace Additionally, is it concerning that when creating Scala from v 3.5.0, an exception is thrown when after generate is called on the model "cdm.product.template?"

You raise some valid concerns. We currently lack docs on this - that's something which we want to improve in the nearby future. I will try to get this prioritised. In the meantime, I will see if I can concisely describe the process on Tuesday, so you can already continue work.

Regarding the error you're seeing while parsing the CDM: I've internally raised an issue for this. That is probably a bug on the DSL side. Thanks for letting us know.

dschwartznyc commented 1 year ago

@SimonCockx looking forward to more detailed guidance on the process and object structure.

BTW, the version is coming through as "${project.version}" which would seem to be an uninterpolated maven variable.

Not certain whether this is causing the exception noted but when the Scala generator is run against CDM v 3.5.0, one of the names of the Supertypes in ScalaModelObjectGenerator.generate is coming up as NULL causing the sorting to throw an exception.

dcarrascosa00 commented 1 year ago

@SimonCockx Hi Simon, is it possible to import org.eclipse.xtext.testing.util.ParseHelper in PythonCodeGenerator.java? I'm trying to move the functionality of PythonFilesGeneratorTest.xtend to the PythonGenerator file, and I need to use the method parse in it, but the import is unresolved. The TestGenerator and the Generator files come from the same package, "com.regnosys.rosetta.generator.python"

SimonCockx commented 1 year ago

@SimonCockx looking forward to more detailed guidance on the process and object structure.

BTW, the version is coming through as "${project.version}" which would seem to be an uninterpolated maven variable.

The variable is interpolated during the CDM build before code generators are run, so in practice this shouldn't be a problem.

Not certain whether this is causing the exception noted but when the Scala generator is run against CDM v 3.5.0, one of the names of the Supertypes in ScalaModelObjectGenerator.generate is coming up as NULL causing the sorting to throw an exception.

Hm, I think the error you saw shouldn't be related to code generators - scoping (mostly) happens before that, during parsing. I've raised an issue on the DSL project as well: https://github.com/REGnosys/rosetta-dsl/issues/532.

@SimonCockx Hi Simon, is it possible to import org.eclipse.xtext.testing.util.ParseHelper in PythonCodeGenerator.java? I'm trying to move the functionality of PythonFilesGeneratorTest.xtend to the PythonGenerator file, and I need to use the method parse in it, but the import is unresolved. The TestGenerator and the Generator files come from the same package, "com.regnosys.rosetta.generator.python"

Hi @dcarrascosa00. The class ParseHelper should only be used for testing, which is why you are only able to import it in test classes. The second argument of a code generator (List<RosettaRootElement> elements) already contains a list of parsed elements - it's not the responsibility of a code generator. Would you be able to translate all functionality using this parameter instead?

Hugo and I have raised a couple of issues on the DSL repository that aim to improve our code generation infrastructure. Hopefully addressing those will make the integration easier. I'm looking forward to hearing from you during our meeting next week!

dschwartznyc commented 1 year ago

Checking in. Have we set a time for the meeting next week?

On May 3, 2023 at 8:03 AM, <SimonCockx @.***)> wrote:

@SimonCockx (https://github.com/SimonCockx) looking forward to more detailed guidance on the process and object structure.

BTW, the version is coming through as "${project.version}" which would seem to be an uninterpolated maven variable.

The variable is interpolated during the CDM build before code generators are run, so in practice this shouldn't be a problem.

Not certain whether this is causing the exception noted but when the Scala generator is run against CDM v 3.5.0, one of the names of the Supertypes in ScalaModelObjectGenerator.generate is coming up as NULL causing the sorting to throw an exception.

Hm, I think the error you saw shouldn't be related to code generators - scoping (mostly) happens before that, during parsing. I've raised an issue on the DSL project as well: REGnosys/rosetta-dsl#532 (https://github.com/REGnosys/rosetta-dsl/issues/532).

@SimonCockx (https://github.com/SimonCockx) Hi Simon, is it possible to import org.eclipse.xtext.testing.util.ParseHelper in PythonCodeGenerator.java? I'm trying to move the functionality of PythonFilesGeneratorTest.xtend to the PythonGenerator file, and I need to use the method parse in it, but the import is unresolved. The TestGenerator and the Generator files come from the same package, "com.regnosys.rosetta.generator.python"

Hi @dcarrascosa00 (https://github.com/dcarrascosa00). The class ParseHelper should only be used for testing, which is why you are only able to import it in test classes. The second argument of a code generator (List elements) already contains a list of parsed elements - it's not the responsibility of a code generator. Would you be able to translate all functionality using this parameter instead?

Hugo and I have raised a couple of issues on the DSL repository that aim to improve our code generation infrastructure. Hopefully addressing those will make the integration easier. I'm looking forward to hearing from you during our meeting next week!

— Reply to this email directly, view it on GitHub (https://github.com/REGnosys/rosetta-code-generators/pull/204#issuecomment-1532994965), or unsubscribe (https://github.com/notifications/unsubscribe-auth/ADD3X6AMBU6PQKNKGHEFFHLXEJJQRANCNFSM6AAAAAAXHA7TQQ). You are receiving this because you were mentioned.Message ID: @.***>

hugohills-regnosys commented 1 year ago

Replaced by https://github.com/REGnosys/rosetta-code-generators/pull/215