Curts0 / PyTabular

Connect to Tabular Models via Python
https://curts0.github.io/PyTabular/
MIT License
67 stars 11 forks source link

Added the option get cultures of a model. #57

Closed Daandamhuis closed 1 year ago

Daandamhuis commented 1 year ago

This option should allow me to get the Cultures for the model. I don't even know if it's according to standards, since this is the first time contributed to any repository.

44 > Comment about Cultures.

Curts0 commented 1 year ago

Hey @Daandamhuis awesome and thank you for the pull request! This is definite need for the package, I'm glad you are getting this in. Would you be able to add a translation to the AdventureWorks Sales.pbix file? It lives in test\adventureworks. That way we can build some pytests off of the new translation for some stability in the feature.

Standards are loose right now until I get some pre-commits implemented, so that's on me until I can get it clear and easy to understand. But for now, I'm just running black and flake8 before merging to main branch. I can run that for us and get it passing.

I'm also going to tweak some of the files a bit and add a PyObjectTranslations class to what you've done. Thanks again for submitting this, I don't work with cultures too often, which is why I have neglected it. It's an obvious need, so I feel like we should be able to get this in quickly.

Daandamhuis commented 1 year ago

I've added some translations to the model, both in Dutch and English and on the most common objects

Daandamhuis commented 1 year ago

@Curts0 : I've added some more "features", maybe should be part of someplace else. But I wanted to include them for documentation purposes.

Curts0 commented 1 year ago

Sent a pull request to your fork.

Getting those implemented should help for the future. One thing to keep in mind is I've got a .bat file that will run the pytest through python version 3.6 to 3.10. I don't think we have any issues yet, but just something to keep in my so it stays compatible with those version.

Daandamhuis commented 1 year ago

@Curts0: Your changes have been merged. Next time I'll create a branch in this repo, probably be a bit easier.

Curts0 commented 1 year ago

Okay awesome and this looks good. I'm away from my computer for today, but I'll get this merged this weekend so we can keep the pull request small. Also, I will build some tests and get all of this published for the next version.