tableau / document-api-python

Create and modify Tableau workbook and datasource files
https://tableau.github.io/document-api-python/
MIT License
331 stars 177 forks source link

Updating field values and adding aliases/calculated fields #103

Open KernpunktAnalytics opened 8 years ago

KernpunktAnalytics commented 8 years ago

Hello everyone,

I have the following requirements:

Since the current version of document-api-python doesn't seem to support this, I'm in the process of forking this repository and implementing those features. I have currently implemented a first version of requirements 1 and 2, and will probably have time for the calculated fields and polishing (aka tests) at some point next week.

Changes are currently in the branch "feature_modify_fields" in my fork. I'd love to hear some feedback about the implementation and the general direction I'm going with this fork. If there is any interest in those features, I'd be happy to start the internal process of getting the CLA underway.

Greetings from Germany

graysonarts commented 8 years ago

Hi!

I checked out your fork/branch, and overall it looks great! I made one comment on changes that would be good to make (it's more around best practices we found while developing the server-client-python library) that would make some of your setters a little easier to maintain.

I'd love to see you submit this has a PR to make it an official part of the document api.

One other thing that would be great for the PR is if you included a couple of tests that exercise the functionality. We rely very heavily on unit tests to ensure we don't inadvertently break things, and given some of the refactoring and redesign I'm doing on the way we handle the xml, it would be good to have those safe guards in place to make sure I don't accidentally break what you've written.

-Russell

KernpunktAnalytics commented 7 years ago

Hi Russel,

thanks for the feedback! I'll make sure to include the input validation decorator as you've mentioned. Test cases are already written, but need some cleanup as well.

I'll send a PR once the CLA clears and the adaptions/enhancements are implemented. Probably end of this/start of next week, hope that's fine with you.

Cheers Felix

(Edit) Quick update: Decorator and test cases are implemented on my fork, excluding calculated fields as this is still WIP. CLA has been sent to github@tableau.com.

graysonarts commented 7 years ago

Hi Felix,

Yay! I'll follow up with the people who handle the CLA stuff, and take a look at your fork. Next week is totally fine, or even the week afterward. Most of the team here are going to be at our US conference next week, so responses will probably be slower than usual since we have a ton to do (and talks to give!)

Thanks for this change! I'm excited to get it into the code base.

KernpunktAnalytics commented 7 years ago

Hey Russel,

thanks for the response! As of now I'm done with the bulk part of my additions, including the tests. I'd love to hear your feedback on it (no rush - enjoy your conference!).

My last commit also adds a basic implementation of the ability to create and modify folders (as its something we currently need for a project), but this will certainly require some more thought/work to get it right (such as maybe creating a dedicated "Folder" class).

Anyways, have fun in Austin!

Cheers Felix

t8y8 commented 7 years ago

@KernpunktAnalytics once you have the CLA signed, feel free to open a PR -- it's just a little easier for us to read the changes and offer feedback over reading the commit log in your fork (Well, at least for me, lots of people are smarter than I) :)

(Thanks so much for doing this!)

Can you open an issue for the Folder Support? We can discuss any design ideas/questions there.

KernpunktAnalytics commented 7 years ago

Hey Tyler,

thanks for the feedback. The CLA is signed & sent and I'm currently waiting for a response/approval from Tableau.

Nevertheless, I'll follow your advice to open a bunch of dedicated Issues/PRs for the things I'm currently working on. Just don't merge them until the CLA has been cleared on your site ;)

Enjoy your weekend, Felix

KernpunktAnalytics commented 7 years ago

Piggybacking on this thread to ping @t8y8 and @RussTheAerialist : Have there been any news regarding the CLA? I've sent it out on the 2nd and haven't heard back since then. If you need any more information please let me know :)

t8y8 commented 7 years ago

@KernpunktAnalytics apologies on the delay, both @RussTheAerialist and I are on vacation.

@benlower or @LGraber should be able to help!

KernpunktAnalytics commented 7 years ago

Ah sorry, I didn't intend to interrupt your vacation. Thanks for the pinging the appropriate substitutes, enjoy your vacation!