New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove 'variations' field as it is redundant #64

Closed
Sembiance opened this Issue Aug 3, 2015 · 9 comments

Comments

Projects
None yet
5 participants
@Sembiance
Collaborator

Sembiance commented Aug 3, 2015

I am thinking about removing the 'variations' field. Essentially all it is doing is pointing to other cards in the same set that have the same name. That's it. Seems pretty easy for anyone consuming the MTGJSON data that if multiple cards in the same set have the same name, clearly they are variations of each other.

Also, right now the 'variations' field uses multiverseid's which many sets do not have, thus the field is already absent on a good number of cards and sets. So removing it would actually bring some consistency to the data.

@Sembiance Sembiance added the discuss label Aug 3, 2015

@florentdouine

This comment has been minimized.

Show comment
Hide comment
@florentdouine

florentdouine Aug 3, 2015

Contributor

I agree @Sembiance :) It's consuming too much time to fetch every urls

Contributor

florentdouine commented Aug 3, 2015

I agree @Sembiance :) It's consuming too much time to fetch every urls

@flyingmutant

This comment has been minimized.

Show comment
Hide comment
@flyingmutant

flyingmutant Aug 3, 2015

Yes, sounds like a good idea.

flyingmutant commented Aug 3, 2015

Yes, sounds like a good idea.

@jenjia

This comment has been minimized.

Show comment
Hide comment
@jenjia

jenjia Aug 4, 2015

Since we still need to run trough the array of card to find the others, it doesn't make sense, as you explained. It would make more sense if the array of cards where ordered by cards number, and then the variations field would be that number. This would makes the search instant. (I know that some sets doesn't haver numbers).
(i don't know if i explained myself good, my english is so bad, god dammit)

jenjia commented Aug 4, 2015

Since we still need to run trough the array of card to find the others, it doesn't make sense, as you explained. It would make more sense if the array of cards where ordered by cards number, and then the variations field would be that number. This would makes the search instant. (I know that some sets doesn't haver numbers).
(i don't know if i explained myself good, my english is so bad, god dammit)

@Sembiance Sembiance closed this in a0b9abc Aug 18, 2015

@esb

This comment has been minimized.

Show comment
Hide comment
@esb

esb Aug 20, 2015

This change is troublesome for me. I need an array of alternatives to make it easy to match card versions. Not all systems use the same version numbering so often I have to visually verify different card versions to match things up. I need the alternatives to keep track of this process.

It was useful to know that a card had different versions as I loaded the data, so that I could build the required matching data. With that gone now, it looks like I will have to code up a second pass of the data to try to reconstruct the version data.

It would be really useful to reinstate this data as it makes my processing task a little bit easier. If people don't need the data, then it can just be ignored.

esb commented Aug 20, 2015

This change is troublesome for me. I need an array of alternatives to make it easy to match card versions. Not all systems use the same version numbering so often I have to visually verify different card versions to match things up. I need the alternatives to keep track of this process.

It was useful to know that a card had different versions as I loaded the data, so that I could build the required matching data. With that gone now, it looks like I will have to code up a second pass of the data to try to reconstruct the version data.

It would be really useful to reinstate this data as it makes my processing task a little bit easier. If people don't need the data, then it can just be ignored.

@esb

This comment has been minimized.

Show comment
Hide comment
@esb

esb Aug 20, 2015

Could I also say that it may appear to be redundant data, but the 'variations' field reports important information about a card. It says that this card has multiple versions. With the field gone, it's necessary to do additional processing on the set to determine if the card has multiple versions. This change is removing useful information about a card.

esb commented Aug 20, 2015

Could I also say that it may appear to be redundant data, but the 'variations' field reports important information about a card. It says that this card has multiple versions. With the field gone, it's necessary to do additional processing on the set to determine if the card has multiple versions. This change is removing useful information about a card.

@Sembiance

This comment has been minimized.

Show comment
Hide comment
@Sembiance

Sembiance Aug 20, 2015

Collaborator

You can use the 'imageName' field as a way to determine variations. If it ends with a number, then there are variations.

One of the main problems I had with the variations array as it was is that it used multiverseid's to keep track of the variations. This being gatherer specific and gatherer missing basically every single promo card and several other sets, I didn't like that. I now have an 'id' field per card, so I could use that now in a variations field, but still feel that either using the 'imageName' field to detect variations or doing a pre-pass of the data are both acceptable.

Collaborator

Sembiance commented Aug 20, 2015

You can use the 'imageName' field as a way to determine variations. If it ends with a number, then there are variations.

One of the main problems I had with the variations array as it was is that it used multiverseid's to keep track of the variations. This being gatherer specific and gatherer missing basically every single promo card and several other sets, I didn't like that. I now have an 'id' field per card, so I could use that now in a variations field, but still feel that either using the 'imageName' field to detect variations or doing a pre-pass of the data are both acceptable.

@esb

This comment has been minimized.

Show comment
Hide comment
@esb

esb Aug 20, 2015

The image name is not a reliable field to determine the presence of variations.

For example, some of the Zendikar lands have image name ending in 'a'.

There are also instances of cards without variations having image names ending in digits. For example, the Unglued B.F.M. I also think the Deckmasters set contains a number of cards from Alliances that have variations in that set, but only 1 of the variations is included in Deckmasters but the image name still uses the variation name.

It's not really that important that cards such as promo cards don't have multiverse ids, since the variations would only apply to cards that have the same name within the one set, and the promo sets are unlikely to have any variations like this.

For me this is really a major impact on my code, and I'd rather not have to do major surgery on it just to maintain the existing function. It seems that this change is being made purely for asthetics even though it may break the code for some users. Is there a real problem with leaving it in the code, especially if it is actually being used and the alternative is to create a lot of extra work for anyone using the variations.

esb commented Aug 20, 2015

The image name is not a reliable field to determine the presence of variations.

For example, some of the Zendikar lands have image name ending in 'a'.

There are also instances of cards without variations having image names ending in digits. For example, the Unglued B.F.M. I also think the Deckmasters set contains a number of cards from Alliances that have variations in that set, but only 1 of the variations is included in Deckmasters but the image name still uses the variation name.

It's not really that important that cards such as promo cards don't have multiverse ids, since the variations would only apply to cards that have the same name within the one set, and the promo sets are unlikely to have any variations like this.

For me this is really a major impact on my code, and I'd rather not have to do major surgery on it just to maintain the existing function. It seems that this change is being made purely for asthetics even though it may break the code for some users. Is there a real problem with leaving it in the code, especially if it is actually being used and the alternative is to create a lot of extra work for anyone using the variations.

@Sembiance

This comment has been minimized.

Show comment
Hide comment
@Sembiance

Sembiance Aug 20, 2015

Collaborator

I will be adding the variations field back. This requires rebuilding all the sets which can take an hour or two. I'll update this issue again once the update has been deployed to the site.

Collaborator

Sembiance commented Aug 20, 2015

I will be adding the variations field back. This requires rebuilding all the sets which can take an hour or two. I'll update this issue again once the update has been deployed to the site.

@Sembiance

This comment has been minimized.

Show comment
Hide comment
@Sembiance

Sembiance Aug 21, 2015

Collaborator

Variations have been added back in.

Note: In the future I want to change the multiverseid's to hash id's. See Issue #69

Collaborator

Sembiance commented Aug 21, 2015

Variations have been added back in.

Note: In the future I want to change the multiverseid's to hash id's. See Issue #69

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment