chrthomsen / pygrametl

Official repository for pygrametl - ETL programming in Python
http://pygrametl.org
BSD 2-Clause "Simplified" License
289 stars 41 forks source link

Inconsistencies in documentation. #27

Closed Jonasmadsen closed 3 years ago

Jonasmadsen commented 3 years ago

site: https://chrthomsen.github.io/pygrametl/doc/examples/dimensions.html#typeoneslowlychanging-dimension says: "A slowly changing dimension that is only type 1 is currently not supported." however later it says: "TypeOneSlowlyChangingDimension allows the creation of a Type 1 slowly changing dimension."

site: https://chrthomsen.github.io/pygrametl/doc/examples/dimensions.html#cacheddimension The example code shows: from pygrametl.datasources import CSVSource however it is unused The example code shows: for row in sales: however sales is not defined anywhere.

site: https://chrthomsen.github.io/pygrametl/doc/quickstart/beginner.html The example code shows: region_source = CSVSource(csvfile=region_file_handle, delimiter=',') however csvfile appears renamed to f (not a great name) ???

In generel it appears random if imports are included or not in the example code.

I get that these are minor issues, but when I try to learn a new framework/library it is essential that the documentation, especially the quickstart-guide, doesn't contain:

I really like this project and think better documentation could have helped me learn pygrametl faster. If this is not the correct forum for this feedback please let me know and I will delete my issue.

Best Regards Jonas

chrthomsen commented 3 years ago

Hello,

Thank you for your feedback. We appreciate it and think this the right place to share it!

The first two issues should be easy to fix.

The third issue about csvfile or f is a problem coming from standard Python since CSVSource just points to csv.DictReader. The documentation for DictReader says this argument is called csvfile but the implementation actually uses f so we have to use f (and the guide should then of course be updated to show the correct information).

Imports should be included in the code.

We will try to fix these things when we have time to do so. If you have the time and energy, we would also welcome a pull-request which fixes this.

Best regards, Christian Thomsen